| msg192484 - (view) |
Author: Jonathan Eldridge (jveldridge) |
Date: 2013-07-06 20:14 |
| With the following directory structure: computer$ ls foo/ computer$ ls foo/ bar/ foo_file.txt computer$ ls foo/bar/ bar_file.txt Running: os.path.relpath('foo/bar/bar_file.txt', 'foo/foo_file.txt') Returns: '../bar/bar_file.txt' But should return: 'bar/bar_file.txt' |
|
|
| 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 posixpath.py and ntpath.py 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) *  |
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) * |
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)  |
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. http://hg.python.org/cpython/rev/70837970c5d8 New changeset 7de05609e390 by R David Murray in branch 'default': #18389: Clarify that relpath does not access the file system. http://hg.python.org/cpython/rev/7de05609e390 New changeset 1345d8dbcb19 by R David Murray in branch '2.7': #18389: Clarify that relpath does not access the file system. http://hg.python.org/cpython/rev/1345d8dbcb19 |
|
|
| msg192978 - (view) |
Author: R. David Murray (r.david.murray) * |
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. :) |
|
|