[Patches] [ python-Patches-1339796 ] new function: os.path.relpath

[SourceForge.net]

Patches item #1339796, was opened at 2005-10-27 11:23
Message generated for change (Comment added) made by nnorwitz
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1339796&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Library (Lib)
Group: Python 2.5
Status: Open
Resolution: None
Priority: 5
Submitted By: Richard Barran (rbarran)
Assigned to: Nobody/Anonymous (nobody)
Summary: new function: os.path.relpath

Initial Comment:
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 :-)

----------------------------------------------------------------------

>Comment By: Neal Norwitz (nnorwitz)
Date: 2005-12-19 10:00

Message:
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.

----------------------------------------------------------------------

Comment By: Richard Barran (rbarran)
Date: 2005-12-19 03:14

Message:
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.


----------------------------------------------------------------------

Comment By: Reinhold Birkenfeld (birkenfeld)
Date: 2005-12-17 09:04

Message:
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?

----------------------------------------------------------------------

Comment By: Richard Barran (rbarran)
Date: 2005-10-31 04:54

Message:
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

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2005-10-27 23:21

Message:
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.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1339796&group_id=5470