Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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-100479: Add optional
blueprint
argument topathlib.PurePath
#100481GH-100479: Add optional
blueprint
argument topathlib.PurePath
#100481Changes from 6 commits
a6fdd0e
b061747
99eb8b1
4759d01
ef6f4c3
595b8ae
dcfe70a
117fe4b
e75dedc
5a6bd3f
f2f1048
3c172fb
4637109
ae48454
e7a8fe3
7f12faa
687c764
d7e326a
a65d499
d4b15d7
958b183
1e10188
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Hmm, wouldn't this be simpler as a classmethod?
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.
Making it a classmethod means that you can't share state between paths no?
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 see -- sorry, just getting back to pathlib stuff after a while away!
I guess I find the "makepath" name quite unintuitive in that case. The name to me implies that it's "just" an alternative constructor, which I'd expect to be a classmethod. Maybe something like "sproutpath" (feel free to bikeshed the name), which is clearer that it is using the state of the current
Path
instance would be better?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.
with_segments()
, perhaps? Complementswith_name()
etc.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.
with_segments
sounds like it's kiiinda "the previous path's segements, but with some new ones added at the end".What about "newpath"? I think that reinforces the fact that the returned object has completely different segments to the old one.
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.
Although this method is mostly called under-the-hood, it may also be useful when dealing with subclasses of pathlib classes. For example:
I mention this just in case it helps us with the naming question. In the above example,
readme_path.newpath()
is a sort of factory for path objects sharing the same underlyingTarFile
object. And I suppose factories make things. But I don't mindnewpath()
:)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.
Of the options floated so far, I like the "newpath" name the best. But it's your module now, afterall, so I don't feel like I should have the final call here :D
You could maybe start a thread on Discord or Discuss if you'd like the opinion of more core devs?
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.
Thread created here! https://discuss.python.org/t/bikeshedding-opportunity-help-name-a-pathlib-method/26186