gh-117114: Same interface for `posixpath` & `ntpath`

[nineteendo]

• Issue: gh-117114

[bedevere-app[bot]]

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

[zware]

There are quite a number of failing tests on multiple platforms (all of them, actually :)) that you'll need to address for this to go anywhere.

I'm not entirely clear on what your goal with this is, though. It may be more worth while to flesh out the issue with some discussion of what you want to change, why, who it will help, and the risks. The issue currently lists several things that can change, but is very light on details about why they should :)

[bedevere-app[bot]]

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]

It looks like ntpath.splitroot(), posixpath.dirname() and posixpath.basename() will be made slower by this patch. IMO that requires better motivation than code-deduplication. Happy to be shown wrong by some benchmarks :)

edit: some simple benchmarks:

$ ./python -m timeit -s "from ntpath import splitroot" "splitroot('c:/foo')"  
500000 loops, best of 5: 504 nsec per loop  # before
500000 loops, best of 5: 723 nsec per loop  # after
# --> 1.44x slower

$ ./python -m timeit -s "from posixpath import dirname" "dirname('/etc/hosts')"  
500000 loops, best of 5: 410 nsec per loop  # before
500000 loops, best of 5: 533 nsec per loop  # after
# --> 1.3x slower

$ ./python -m timeit -s "from posixpath import basename" "basename('/etc/hosts')"  
1000000 loops, best of 5: 261 nsec per loop  # before
 500000 loops, best of 5: 537 nsec per loop  # after
# --> 2.06x slower

[barneygale]

Is splitroot() faster now? If so, please could you provide some benchmarks?

[nineteendo]

@barneygale, Is this acceptable?

$ python -m timeit -s "from splitroot import splitroot1" "splitroot1('c:/foo')"
500000 loops, best of 5: 574 nsec per loop  # before
$ python -m timeit -s "from splitroot import splitroot2" "splitroot2('c:/foo')"
500000 loops, best of 5: 694 nsec per loop  # after
# --> 1.21x slower

[barneygale]

Why are you changing splitroot() then? I don't get it. It's not significantly faster nor significantly clearer as far as I can tell. What am I missing?

[nineteendo]

Sorry for wasting your time. I thought it would make the code cleaner by splitting the drive and root in 2 steps.

I believe that should resolve all issues. It's a bit unfortunate that after 7 commits I only added a function that always returns False and removed one duplicate function.

[nineteendo]

The docs still need to be updated to reflect the changes: https://docs.python.org/3.13/library/os.path.html#os.path.isdevdrive

Changed in version 3.13: Added Unix support.

[zooba]

The docs still need to be updated to reflect the changes

I would be clear that it's present on POSIX, and that testing for the function is no longer a good way to determine whether the platform supports it at all. It doesn't have Unix "support", it is just now present and always returns False.

It's a bit unfortunate that after 7 commits I only added a function that always returns False

It happens to all of us at times :-) It would be more unfortunate to have merged changes that aren't necessary or helpful.

[nineteendo]

I would be clear that it's present on POSIX, and that testing for the function is no longer a good way to determine whether the platform supports it at all. It doesn't have Unix "support", it is just now present and always returns False.

I was talking about the availability of isdevdrive: only Windows for 3.12, unless it's back ported?

[zooba]

Right, but availability of the function is not support of the feature. Saying that we've "Added Unix support" implies the latter, when there's no support to add as this is (currently?) a Windows-only feature. (If we said the same thing when isjunction was added to posixpath, then we shouldn't have said it like that.)

Better text would be: "Changed in version 3.13: The function is now available on all platforms, and will always return False on those that have no support for Dev Drives"

[nineteendo]

You can now check if a platform supports Dev Drives / junctions (this also deduplicates the fallback implementations):

• ntpath.isdevdrive is not genericpath.isdevdrive, posixpath.isdevdrive is not genericpath.isdevdrive
• ntpath.isjunction is not genericpath.isjunction, posixpath.isjunction is not genericpath.isjunction

[nineteendo]

Done, I removed the redundant availabilities from the documentation, and also added a link to genericpath.py. Also, @zware, I have made the requested changes; please review again

[bedevere-app[bot]]

Thanks for making the requested changes!

@zware: please review the changes made to this pull request.

[zooba]

Thanks for the contribution!

[bedevere-app[bot]]

GH-117249 is a backport of this pull request to the 3.12 branch.

[nineteendo]

It would be convenient if posixpath.isdevdrive was also available on 3.12, see the PR above.

[zooba]

Convenient, but this isn't a bug. The best way to handle 3.12 and earlier is to test for whether the function exists, and post-3.13 (when developers drop 3.12 support) is only a year after that. It's not worth breaking our backporting rules to gain one more year of testing for AttributeError.

[nineteendo]

Understood, os.path.isdevdrive only existing on Windows on 3.12 is not considered a bug and therefore this can't be backported.