New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gh-101000: Add os.path.splitroot() #101002
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drive-by comments from seeing this PR on discourse, feel free to disregard if I'm saying/asking something stupid.
Thanks for the review! All good feedback I think! |
Co-authored-by: Eryk Sun <eryksun@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm persuaded that this is a good idea. Thanks for working on this!
Here's a docs review. Haven't got to looking at the implementation yet (will do soon).
Co-authored-by: Eryk Sun <eryksun@gmail.com>
Co-authored-by: Eryk Sun <eryksun@gmail.com>
>>> splitroot('/etc/hosts') | ||
('', '/', 'etc/hosts') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
>>> splitroot('/etc/hosts') | |
('', '/', 'etc/hosts') | |
>>> splitroot('home/barney') | |
('', '', 'home/barney') | |
>>> splitroot('/home/barney') | |
('', '/', 'home/barney') | |
>>> splitroot('//home/barney') | |
('', '//', 'home/barney') | |
>>> splitroot('///home/barney') | |
('', '/', '//home/barney') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps getting too long IMHO. @AlexWaygood what do you think? (and below)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example with a relative path could be skipped as something most people will understand intuitively. I think it's helpful to show "/" vs "//" vs "///", or at least "//" vs "///". I think the result with "//" may be surprising because Linux and macOS handle it the same as "/" and "///".
>>> splitroot('Windows/notepad') | ||
('', '', 'Windows/notepad') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
>>> splitroot('Windows/notepad') | |
('', '', 'Windows/notepad') | |
>>> splitroot('Users/Barney') | |
('', '', 'Users/Barney') | |
>>> splitroot('/Users/Barney') | |
('', '/', 'Users/Barney') |
Co-authored-by: Eryk Sun <eryksun@gmail.com>
This PR introduces
os.path.splitroot()
. See #101000 for motivation.In
ntpath
, the implementation derives fromsplitdrive()
. Thesplitdrive()
function now callssplitroot()
, and returnsdrive, root + tail
. Other functions now callsplitroot()
rather thansplitdrive()
. In most cases this replaces their own parsing of path roots. It also avoids adding a stack frame.In
posixpath
, thenormpath()
function now callssplitroot()
rather than parsing path roots itself.In
pathlib
, path constructors now callsplitroot()
rather than using a slow OS-agnostic implementation. Performance:Future work:
nt._path_splitroot()