homepage

The docs say that "the return value is the concatenation of path1, and optionally path2, etc., with exactly one directory separator (os.sep) inserted between components, unless path2 is empty."
But os.path.join('x','') returns 'x/' in which path1 and path2 *are* separated by exactly one os.sep, even though path2 is empty.
Either the docs or the implementation should be updated.

Since changing the implementation would be a backward incompatible behavior change to a behavior that has existed for a long time, it's the docs that should be updated.

The behaviour is a bit more nuanced:
>>> os.path.join('x', '')
'x/'
>>> os.path.join('x', '', 'y')
'x/y'
>>> os.path.join('x', '', 'y', '', '')
'x/y/'
>>> os.path.join('', 'x')
'x'
I'm unsure how to word this best, maybe "non-trailing empty paths are ignored"?

I would say something like the following.
The function join(path1, path2) is almost like os.sep.join(path1, path2), but (1) trailing path separators in path1 are ignored and (2) the result is simply path2 when path2 is an absolute path. The call join(path1, path2, path3) is equivalent to join(join(path1, path2), path3), and similarly for more than three paths.

That doesn't cover the os.path.join('', 'x') case, and I'm not sure it makes os.path.join('x//', 'y') clear - though that doesn't matter as much.
How about making (2) "the result is simply path2 when path1 is empty or path2 is an absolute path?

I think Brian's second solution ("the result is simply path2 when path1 is empty or path2 is an absolute path?") is a strong one. If that were tacked on towards the end it would add some clarity to the docs for people who will end up using this behavior or want a more in-depth explanation. At the same time, I think putting it towards the end (as more of a side note, like the bit about behavior on Windows with drive names) lets less sophisticated users (like me) ignore that piece of documentation.

Realizing I still don't know what os.join.path does, I looked at the source. The comment in posixpath.py is:
# Ignore the previous parts if a part is absolute.
# Insert a '/' unless the first part is empty or already ends in '/'.
I find this clear and it directly corresponds to the implementation.
On the other hand, the source of ntpath.join() is a nightmare, and there's no similarly simple comment there.

Radu, while the comments are not as clear for ntpath, the behavior is the same. So, the comment you detailed from posixpath could be adapted to Windows by replacing '/' with 'a separator' or something of that nature.
That said, the comment in posixpath could be adapted to clear english like so:
join() inserts a separator unless the first part is empty or already ends in a separator. If a part is absolute, join() ignores the previous parts.

I think the comment is fine as is. +1 to adding your wording to the docs.

"first part" by itself sounds like there can only be two parts. How about 'inserts a separator between each pair of...'
Also, what does 'absolute' mean on Windows? Does it include the drive? If so, the second sentence should probably say 'if a part starts with a separator...' (Assuming, of course, that that's how ntpath.join actually works).

Comment in ntpath.isabs:
For Windows it is absolute if it starts with a slash or backslash (current volume), or if a pathname after the volume-letter-and-colon or UNC-resource starts with a slash or backslash.

Here is a patch that I think describes the algorithm correctly, based on the comments in the module, with a clarifying parenthetical to cover the non-obvious consequence of that algorithm.

David's change sounds good to me.

New changeset 1e89444f4ebc by R David Murray in branch '2.7':
#9921: clarify os.path.join joining algorithm
http://hg.python.org/cpython/rev/1e89444f4ebc
New changeset f5f5b715be7e by R David Murray in branch '3.2':
#9921: clarify os.path.join joining algorithm
http://hg.python.org/cpython/rev/f5f5b715be7e
New changeset b6759568b812 by R David Murray in branch 'default':
merge #9921: clarify os.path.join joining algorithm
http://hg.python.org/cpython/rev/b6759568b812