Match docstring and args for `posixpath.join` and `ntpath.join` #104478

brl0 · 2023-05-14T16:37:24Z

Adpated docstring from posixpath.join to ntpath.join, which was previously blank.

Also renamed arguments to posixpath.join to match ntpath.join. This is a cosmetic change, but I think they should have matching signatures since they are used interchangeably by os.path and pathlib.

Here are the current signatures:

function	signature
posixpath.join	def join(a, *p)
ntpath.join	def join(path, *paths)

This change is not much more than fixing a typo, so I didn't open an issue for this, but would be willing to do so if that is preferred.
Also, this change is probably not news-worthy, but I can add an entry if needed.

cpython-cla-bot · 2023-05-14T16:37:51Z

All commit authors signed the Contributor License Agreement.

barneygale · 2023-05-14T17:54:49Z

Lib/ntpath.py

+    If any component is an absolute path, all previous path components
+    will be discarded.  An empty last part will result in a path that


In ntpath, "absolute path" means a path with a root, such as "\bar". If this path does not have a drive letter, then the previous drive letter (if any) is retained. So it's not right to say that all previous components will be discarded -- it's a little more subtle than that!

Thanks for the clarification! I made a change to be more descriptive, let me know if it still needs work.

Lib/posixpath.py

bedevere-bot · 2023-05-14T18:11:38Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

barneygale · 2023-05-14T18:14:23Z

Adding a docstring for os.path.join() on Windows is worthy of an issue and a news entry, IMO.

brl0 · 2023-05-15T00:24:06Z

@barneygale,

Thanks for your quick review and feedback, it is much appreciated. I have made the requested changes; please review again.

Also, thanks for your great work on pathlib. I have been contributing to a project universal_pathlib, based on fsspec, and it was in that context I was digging around some of your changes in cpython to begin with. I was trying to figure out what would need to be adapted to properly implement a new path flavour module for classes inheriting from pathlib, which is why I was looking closely at docstrings and function args.

arhadthedev · 2023-05-15T02:04:35Z

Do we need a news entry for documentation?

brl0 added 2 commits May 14, 2023 16:25

Rename posixpath.join args to match ntpath.join

6af1da7

Add docstring to ntpath.join

70626f5

bedevere-bot added the awaiting review label May 14, 2023

This comment was marked as outdated.

Sign in to view

Merge remote-tracking branch 'upstream/main' into sync_join_funcs

d6bc11b