Skip to content

Conversation

@barneygale
Copy link
Contributor

@barneygalebarneygale commented Dec 7, 2024

Remove Parser.sep, which is the protocol's only non-method member. Instead, we compute the separator from PathBase.__init_subclass__() by calling parser.join('a', 'a').strip('a'), and store the result on the subclass as cls._sep. We also set cls._case_sensitive according to parser.normcase('Aa') == 'Aa', which was previously done in a caching function.

Remove `ParserBase.sep`, which is the class's only non-method member. Instead, we compute the separator from `PathBase.__init_subclass__()` by calling `parser.join('a', 'a').strip('a')`, and store the result on the subclass as `cls._sep`. We also set `cls._case_sensitive` according to `parser.normcase('Aa') == 'Aa'`, which was previously done in a caching function.
@encukou
Copy link
Member

I see there's currently no distinction between pathlib-internal attribute names and names that are free for third-party subclasses to use.
Should there be a stronger namespace for these, like _pathlib_sep, _pathlib_globber & _pathlib_stack?
Or should _sep & co. be documented so users know to not override them?

@barneygale
Copy link
ContributorAuthor

I suppose another option is to make them public as PurePath.separator and PurePath.is_case_sensitive

@barneygalebarneygale changed the title GH-127456: pathlib ABCs: remove ParserBase.sepGH-127456: pathlib ABCs: remove Parser.sepDec 9, 2024
@encukou
Copy link
Member

Hmm. What's the reason for removing sep from Parser, again? That's really the correct place for it.

Should there be a stronger namespace for these, like _pathlib_sep, _pathlib_globber & _pathlib_stack?

I guess that might be for another PR, since it affects _globber and methods as well.

I suppose another option is to make them public as PurePath.separator and PurePath.is_case_sensitive

Right, but then __init_subclass__ should only set them if they're not in __dict__.
It's probably best to keep them as private attributes. Or continue using @functools.cache to avoid magic leaking into the API -- e.g. here, you can't do:

classMyPath(PurePathBase): passMyPath.parser=MyParser()

@barneygale
Copy link
ContributorAuthor

All good points. Really my only motivation was to make the Parser interface method-only - partly so that issubclass() checks work, and partly for likely-misguided aesthetic reasons. You've convinced me it isn't a good idea, thank you for your feedback as ever!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@barneygale@encukou