Title: document that os.path.relpath does not interrogate the file system
Type: behavior Stage: resolved
Components: Documentation Versions: Python 3.3, Python 3.4, Python 2.7
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: docs@python Nosy List: docs@python, jveldridge, madison.may, python-dev, r.david.murray
Priority: normal Keywords: patch

Created on 2013-07-06 20:14 by jveldridge, last changed 2013-07-12 22:28 by madison.may. This issue is now closed.

File name Uploaded Description Edit
relpath.patch madison.may, 2013-07-06 22:06 Test cases + patch to handle paths to files with extensions as arguments for 'start'. review
relpath_v2.patch madison.may, 2013-07-07 00:21 Adds isfile(start) check and corresponding test cases, but currently causes test_unittest to fail. review
Issue18389_3-4.patch madison.may, 2013-07-11 18:03 Documentation added to clear up confusion around behavior of relpath review
Messages (12)
msg192484 - (view) Author: Jonathan Eldridge (jveldridge) Date: 2013-07-06 20:14
With the following directory structure:

    computer$ ls
    computer$ ls foo/
    bar/  foo_file.txt
    computer$ ls foo/bar/


    os.path.relpath('foo/bar/bar_file.txt', 'foo/foo_file.txt')



But should return:

msg192489 - (view) Author: Madison May (madison.may) * Date: 2013-07-06 21:55
So the problem arises because a path to a file (not a path to a directory) is passed as an argument to start. So the way I see it, we could go one of several directions: 

1) We could check for the presence of an extension in the start argument, and truncate start_path to start_path[:-1] if an extension is found.  This wouldn't deal with the case where a path to a file without an extension is passed to relpath(), but it would deal with the large majority of cases.  I'm in favor of this option. 

2) We could add a warning to the docs and let users know that paths to files shouldn't be passed as the 'start' argument to relpath() -- only paths to directories are valid.  You could perhaps even raise an error when a path to a file with an extension is passed as the 'start' argument.

I've attached a patch for and with the first option in mind.  The patch also contains an test case for this behavior.
msg192491 - (view) Author: Madison May (madison.may) * Date: 2013-07-06 22:06
Whoops -- I forgot to actually upload the patch.  Here it is.
msg192492 - (view) Author: Jonathan Eldridge (jveldridge) Date: 2013-07-06 22:07
Seems like you could also narrow the case where the incorrect behavior occurs for files without an extension by doing a os.path.isfile check as well--that will only return True for cases where the file exists, but then  relpath could work correctly for all existing files and all files that have an extension.
msg192503 - (view) Author: Madison May (madison.may) * Date: 2013-07-07 00:21
That could definitely be beneficial.  I've attached a second patch with that suggestion in mind.  However, note that test_unittest is failing after adding the isfile(start) check. I think its related to the large amount of mocking that's happening in test_unittest: os.path.isfile has been reassigned in each test case that fails.  I haven't managed to wrap my head around it yet, but feel free to test it out for yourself.
msg192753 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-07-09 14:12
I understand your concern, but the API is that 'start' is a directory.  The function does not interrogate the file system, and should not do so.  It is purely a path computation, and as such the current behavior is correct.
msg192754 - (view) Author: Jonathan Eldridge (jveldridge) Date: 2013-07-09 14:21
At the very least, the documentation should be updated to explain this.
msg192761 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-07-09 16:15
That's a good point.
msg192884 - (view) Author: Madison May (madison.may) * Date: 2013-07-11 18:03
Patch for 3.4 added.  I tried to keep things short and sweet.
msg192975 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-07-12 22:21
New changeset 70837970c5d8 by R David Murray in branch '3.3':
#18389: Clarify that relpath does not access the file system.

New changeset 7de05609e390 by R David Murray in branch 'default':
#18389: Clarify that relpath does not access the file system.

New changeset 1345d8dbcb19 by R David Murray in branch '2.7':
#18389: Clarify that relpath does not access the file system.
msg192978 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-07-12 22:23
Thanks, Madison.  I tweaked the wording slightly, but the essence is the same :).
msg192980 - (view) Author: Madison May (madison.may) * Date: 2013-07-12 22:28
Thanks, David.  I like your changes -- sounds a lot cleaner and more explicit. :)
Date User Action Args
2013-07-12 22:28:23madison.maysetmessages: + msg192980
2013-07-12 22:23:06r.david.murraysetstatus: open -> closed
resolution: fixed
messages: + msg192978

stage: needs patch -> resolved
2013-07-12 22:21:54python-devsetnosy: + python-dev
messages: + msg192975
2013-07-11 18:03:41madison.maysetfiles: + Issue18389_3-4.patch

messages: + msg192884
2013-07-09 16:15:20r.david.murraysetstatus: closed -> open

assignee: docs@python
stage: resolved -> needs patch
title: os.path.relpath gives incorrect results if start parameters is not a directory -> document that os.path.relpath does not interrogate the file system
nosy: + docs@python
versions: + Python 3.3, Python 3.4
messages: + msg192761
components: + Documentation, - Library (Lib)
resolution: rejected -> (no value)
2013-07-09 14:21:13jveldridgesetmessages: + msg192754
2013-07-09 14:12:51r.david.murraysetstatus: open -> closed

nosy: + r.david.murray
messages: + msg192753

resolution: rejected
stage: resolved
2013-07-07 00:21:26madison.maysetfiles: + relpath_v2.patch

messages: + msg192503
2013-07-06 22:07:16jveldridgesetmessages: + msg192492
2013-07-06 22:06:10madison.maysetfiles: + relpath.patch
keywords: + patch
messages: + msg192491
2013-07-06 21:55:16madison.maysetnosy: + madison.may
messages: + msg192489
2013-07-06 20:18:25jveldridgesettype: behavior
2013-07-06 20:14:32jveldridgecreate