Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.3k
gh-74033: Fix bug when Path takes and ignores **kwargs#19632
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
remilapeyre left a comment
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.
Thanks for writing the fix for this issue Yurii!
Uh oh!
There was an error while loading.Please reload this page.
Misc/NEWS.d/next/Library/2020-04-18-17-45-03.bpo-29847.Uxtbq0.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Co-Authored-By: Rémi Lapeyre <remi.lapeyre@henki.fr>
remilapeyre left a comment
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.
Thanks!
remilapeyre left a comment
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.
Hi@uriyyo, as I indicated on bpo,PurePath suffers from a similar issue, it and its subclasses cannot accept**kwargs.
Could you make the same changes toPurePath so the behaviour is the same for the whole module?
Hi@remilapeyre |
Hi@remilapeyre |
remilapeyre left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
Thanks@uriyyo, I just tested your changes and it looks good to me 👍
I just proposed two small things
Uh oh!
There was an error while loading.Please reload this page.
Misc/NEWS.d/next/Library/2020-04-18-17-45-03.bpo-29847.Uxtbq0.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Rémi Lapeyre <remi.lapeyre@henki.fr>
Co-authored-by: Rémi Lapeyre <remi.lapeyre@henki.fr>
@remilapeyre I agree with your suggestions, thanks. |
netlifybot commentedDec 6, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
✅ Deploy Preview forpython-cpython-preview canceled.
|
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.
Since "explicit is better than implicit", I think we wantpathlib.Path to raise an error with keyword arguments instead of lettingPurePath silently accept keyword arguments.
Misc/NEWS.d/next/Library/2020-04-18-17-45-03.bpo-29847.Uxtbq0.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
bedevere-bot commentedDec 16, 2022
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 |
Co-authored-by: Brett Cannon <brett@python.org>
I have made the requested changes; please review again |
bedevere-bot commentedDec 17, 2022
Thanks for making the requested changes! @brettcannon: please review the changes made to this pull request. |
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.
Unfortunately we need to ask that this raises a warning for at least 2 releases.
Uh oh!
There was an error while loading.Please reload this page.
bedevere-bot commentedDec 23, 2022
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. |
bedevere-bot commentedDec 24, 2022
Thanks for making the requested changes! @brettcannon: please review the changes made to this pull request. |
Misc/NEWS.d/next/Library/2020-04-18-17-45-03.bpo-29847.Uxtbq0.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Status check is done, and it's a success ✅. |
Thanks for the PR and the patience! |
Uh oh!
There was an error while loading.Please reload this page.
Fix a bug where
Pathtakes and ignores**kwargsby adding toPurePathclass__init__method which can take only positional arguments.Automerge-Triggered-By: GH:brettcannon