Issue1339796
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2005-10-27 18:23 by rbarran, last changed 2022-04-11 14:56 by admin. This issue is now closed.
Files | ||||
---|---|---|---|---|
File name | Uploaded | Description | Edit | |
relpath_patch.diff | rbarran, 2005-10-27 18:23 | |||
relpath.patch | collinwinter, 2007-03-16 03:04 | Updated/tweaked relpath implementation, with docs | ||
test_posixpath.py.diff | collinwinter, 2007-03-21 18:02 | Patch to (hopefully) fix test failures on Windows. |
Messages (15) | |||
---|---|---|---|
msg48928 - (view) | Author: Richard Barran (rbarran) | Date: 2005-10-27 18:23 | |
Hello, This patch adds a 'relpath' function to module os.path as per RFE #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 :-) |
|||
msg48929 - (view) | Author: Neal Norwitz (nnorwitz) * | Date: 2005-10-28 06:21 | |
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. |
|||
msg48930 - (view) | Author: Richard Barran (rbarran) | Date: 2005-10-31 12:54 | |
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 |
|||
msg48931 - (view) | Author: Georg Brandl (georg.brandl) * | Date: 2005-12-17 17:04 | |
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? |
|||
msg48932 - (view) | Author: Richard Barran (rbarran) | Date: 2005-12-19 11:14 | |
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. |
|||
msg48933 - (view) | Author: Neal Norwitz (nnorwitz) * | Date: 2005-12-19 18:00 | |
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. |
|||
msg48934 - (view) | Author: Richard Barran (rbarran) | Date: 2005-12-21 14:05 | |
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. |
|||
msg48935 - (view) | Author: Richard Barran (rbarran) | Date: 2006-01-30 21:54 | |
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. |
|||
msg48936 - (view) | Author: Collin Winter (collinwinter) * | Date: 2007-03-16 03:04 | |
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 |
|||
msg48937 - (view) | Author: Georg Brandl (georg.brandl) * | Date: 2007-03-16 08:24 | |
Since there is an "Availability" remark, I wouldn't be concerned. |
|||
msg48938 - (view) | Author: Collin Winter (collinwinter) * | Date: 2007-03-16 22:16 | |
Checked in as r54419. Thanks for the patch, Richard! |
|||
msg48939 - (view) | Author: Martin v. Löwis (loewis) * | Date: 2007-03-21 16:12 | |
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. |
|||
msg48940 - (view) | Author: Collin Winter (collinwinter) * | Date: 2007-03-21 18:02 | |
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 |
|||
msg48941 - (view) | Author: Martin v. Löwis (loewis) * | Date: 2007-03-23 11:07 | |
The patch works fine, please apply. |
|||
msg48942 - (view) | Author: Collin Winter (collinwinter) * | Date: 2007-03-23 22:26 | |
Checked in as r54556. |
History | |||
---|---|---|---|
Date | User | Action | Args |
2022-04-11 14:56:13 | admin | set | github: 42530 |
2005-10-27 18:23:55 | rbarran | create |