Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new function: os.path.relpath #42530

Closed
rbarran mannequin opened this issue Oct 27, 2005 · 15 comments
Closed

new function: os.path.relpath #42530

rbarran mannequin opened this issue Oct 27, 2005 · 15 comments
Labels
stdlib Python modules in the Lib dir

Comments

@rbarran
Copy link
Mannequin

rbarran mannequin commented Oct 27, 2005

BPO 1339796
Nosy @loewis, @birkenfeld, @birkenfeld
Files
  • relpath_patch.diff
  • relpath.patch: Updated/tweaked relpath implementation, with docs
  • test_posixpath.py.diff: Patch to (hopefully) fix test failures on Windows.
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2007-03-23.22:26:55.000>
    created_at = <Date 2005-10-27.18:23:55.000>
    labels = ['library']
    title = 'new function: os.path.relpath'
    updated_at = <Date 2007-03-23.22:26:55.000>
    user = 'https://bugs.python.org/rbarran'

    bugs.python.org fields:

    activity = <Date 2007-03-23.22:26:55.000>
    actor = 'collinwinter'
    assignee = 'collinwinter'
    closed = True
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2005-10-27.18:23:55.000>
    creator = 'rbarran'
    dependencies = []
    files = ['6845', '6846', '6847']
    hgrepos = []
    issue_num = 1339796
    keywords = ['patch']
    message_count = 15.0
    messages = ['48928', '48929', '48930', '48931', '48932', '48933', '48934', '48935', '48936', '48937', '48938', '48939', '48940', '48941', '48942']
    nosy_count = 6.0
    nosy_names = ['loewis', 'nnorwitz', 'georg.brandl', 'georg.brandl', 'collinwinter', 'rbarran']
    pr_nums = []
    priority = 'normal'
    resolution = 'accepted'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue1339796'
    versions = ['Python 2.6']

    @rbarran
    Copy link
    Mannequin Author

    rbarran mannequin commented Oct 27, 2005

    Hello,

    This patch adds a 'relpath' function to module os.path
    as per RFE bpo-1309676 and also this thread:
    http://mail.python.org/pipermail/python-dev/2005-September/056709.html

    Here's a description of this function:
    "relpath(path, start=os.curdir)
    Return a relative filepath to 'path' either from the
    current directory or from a specified 'start' point."

    This patch includes Windows and *nix versions.
    It has been tested on Windows XP and OS/X 10.4.

    Note: there is no 'classic mac' version as I don't know
    that platform :-(

    Changed files are:
    lib/ntpath.py
    lib/test/test_ntpath.py
    lib/posixpath.py
    lib/test/test_posixpath.py

    I'll send a second patch for the documentation shortly.

    As this is my first submission please be gentle with me
    if there are any basic errors :-)

    @rbarran rbarran mannequin closed this as completed Oct 27, 2005
    @rbarran rbarran mannequin assigned collinwinter Oct 27, 2005
    @rbarran rbarran mannequin added the stdlib Python modules in the Lib dir label Oct 27, 2005
    @rbarran rbarran mannequin closed this as completed Oct 27, 2005
    @rbarran rbarran mannequin assigned collinwinter Oct 27, 2005
    @rbarran rbarran mannequin added the stdlib Python modules in the Lib dir label Oct 27, 2005
    @nnorwitz
    Copy link
    Mannequin

    nnorwitz mannequin commented Oct 28, 2005

    Logged In: YES
    user_id=33168

    Thanks for the patch, it's not in bad shape. Please attach
    the doc patch file here. It's easier to have a single patch
    than multiple.

    A couple of things about the patch. Some of these should be
    in PEP-8.

    • <> is deprecated in favor of != (didn't see this doc'd in
      PEP-8 though, maybe it's in another PEP)
    • don't add extra parentheses, e.g.,
      (abspath(start)).split(sep) should be abspath(start).split(sep)
    • I'm not sure it's worth checking that there's a path. I
      noticed that abspath() didn't have a similar check. I
      didn't look for other places, but doubt there is much error
      checking since a reasonable exception should be raised.
    • The for loop is a lot to swallow. It would be easier to
      read if you used a local variable for the min_len IMO.
    • I'd like to see a test case for: relpath("a", "a")
    • I'd like to see test cases in ntpath for paths with a
      drive letter
    • I'd like to see test cases for the exceptions raised in
      ntpath

    Another note that isn't a big deal. It's good that you made
    a single patch file. I think most other developers prefer
    the patch to start one level up. ie, don't start in Lib/,
    include it in the patch. I certainly prefer it this way so
    I don't have to cd much. It just makes development a little
    easier.

    You can attach new versions to this tracker item, but try to
    remember to add a description that says version # so no one
    reviews an older version.

    @rbarran
    Copy link
    Mannequin Author

    rbarran mannequin commented Oct 31, 2005

    Logged In: YES
    user_id=1207189

    Hi,

    Thanks for all your comments; I'll amend my code & re-submit
    a patch.
    I've got a few questions for you:

    "
    I'm not sure it's worth checking that there's
    a path. I noticed that abspath() didn't have a similar
    check. I
    didn't look for other places, but doubt there is much
    error checking since a reasonable exception should be raised.
    "

    By adding this check on the input I wanted to avoid this
    error message:

     >>> os.path.relpath('')
    Traceback (most recent call last):
       File "<stdin>", line 1, in <module>
       File
    "/usr/local/cvsrep/python/dist/src/Lib/posixpath.py", line
    473, in relpath
         return os.path.join(*rel_list)
    TypeError: join() takes at least 1 argument (0 given)

    Which to me is obscure and forces you to dive into the
    stdlib code to understand the real cause of the problem.
    I'd noticed that the other functions in os.path don't seem
    to check input, but usually a sensible default value can be
    assumed (example, with abspath: if no argument is given it's
    sensible to use the current dir instead).
    So I'd like to keep the check on the input. However if you
    feel that it's not needed I'll remove it.

    "
    I'd like to see test cases for the exceptions raised in
    ntpath
    "

    When writing this I tried to maintain a consistent style
    with the other tests in the test_ntpath.py script which all
    use the "tester" function. As far as I can tell, this
    function doesn't allow testing of exceptions.
    I was tempted to use Unittest instead (as per the tests I
    wrote for
    posixpath.py) as it would allow testing of exceptions, but
    decided to try and maintain consistency.
    Do you think I should switch to using UnitTest instead?

    Regards, Richard

    @birkenfeld
    Copy link
    Member

    Logged In: YES
    user_id=1188172

    To the OP: have you completed the patch?

    To the others: is this okay to get into 2.5?

    @rbarran
    Copy link
    Mannequin Author

    rbarran mannequin commented Dec 19, 2005

    Logged In: YES
    user_id=1207189

    Most of the patch is completed as per Neal's suggestions for
    improvement.
    I had a few questions outstanding for Neal and depending on
    his advice I was going to modify the input checks and/or the
    unit tests.

    @nnorwitz
    Copy link
    Mannequin

    nnorwitz mannequin commented Dec 19, 2005

    Logged In: YES
    user_id=33168

    Sorry Richard. It's still in my inbox. I'll try to get to
    it soon. Feel free to post any info/questions here so
    others can answer too.

    @rbarran
    Copy link
    Mannequin Author

    rbarran mannequin commented Dec 21, 2005

    Logged In: YES
    user_id=1207189

    Hi all,
    Going on vacation for a few days, will try to finish this
    patch for the new year.

    @rbarran
    Copy link
    Mannequin Author

    rbarran mannequin commented Jan 30, 2006

    Logged In: YES
    user_id=1207189

    Here is a second version of the 'relpath' function.
    I've modified it as per Neal Norwitz's comments, with two
    exceptions:

    • I've left in a check for a path supplied on input, as
      otherwise an unclear exception is raised.
    • I haven't written any test cases for exceptions in
      ntpath.py, as the "tester" function doesn't seem to handle them.

    This function (if accepted) will also require the following
    addition to the documentation:

    relpath(path, start=os.curdir) 
    Return a relative filepath to 'path' either from the current
    directory or from an optional 'start' point.

    @collinwinter
    Copy link
    Mannequin

    collinwinter mannequin commented Mar 16, 2007

    I've attaching a tweaked/updated version of this patch.

    Is anyone bothered that there's no OS/2 or classic Mac implementation for this? If not, I can go ahead and commit it.

    File Added: relpath.patch

    @birkenfeld
    Copy link
    Member

    Since there is an "Availability" remark, I wouldn't be concerned.

    @collinwinter
    Copy link
    Mannequin

    collinwinter mannequin commented Mar 16, 2007

    Checked in as r54419. Thanks for the patch, Richard!

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Mar 21, 2007

    The test fails on Windows. In particular, this test:

    self.assertEqual(posixpath.relpath(os.path.abspath("a")), "a")

    fails. os.path.abspath("a") gives something like r"c:\python26\a". posixpath.relpath is then not able to cope with it. As a result, it returns r"c:\python26\a" as the relative path.

    Using posixpath.abspath doesn't help, either, since that will give r"c:\python26/a" which relpath then still cannot process correctly.

    One solution would be to use pass a POSIX path and start path to relpath; the other would be to use os.path in both cases.
    os.path.relpath(os.path.abspath("a")) does indeed give "a" on Windows.

    @collinwinter
    Copy link
    Mannequin

    collinwinter mannequin commented Mar 21, 2007

    Since the trouble is os.getcwd() returns a non-posix path on Windows, I've taken the approach of mocking os.getcwd() so that it returns something posix-y.

    Martin, could you verify that the attached patch works on Windows?
    File Added: test_posixpath.py.diff

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Mar 23, 2007

    The patch works fine, please apply.

    @collinwinter
    Copy link
    Mannequin

    collinwinter mannequin commented Mar 23, 2007

    Checked in as r54556.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant