Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
miss-islington merged 13 commits intopython:mainfromuriyyo:fix-issue-29847
Jan 14, 2023

Conversation

@uriyyo
Copy link
Member

@uriyyouriyyo commentedApr 21, 2020
edited by AlexWaygood
Loading

Fix a bug wherePath takes and ignores**kwargs by adding toPurePath class__init__ method which can take only positional arguments.

Automerge-Triggered-By: GH:brettcannon

Copy link

@remilapeyreremilapeyre left a 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!

Copy link

@remilapeyreremilapeyre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thanks!

Copy link

@remilapeyreremilapeyre left a 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?

@uriyyo
Copy link
MemberAuthor

Hi@remilapeyre
Sure, I will update this PR.

@uriyyo
Copy link
MemberAuthor

Hi@remilapeyre
I update PR, can you review it?

Copy link

@remilapeyreremilapeyre left a comment
edited
Loading

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

uriyyoand others added2 commitsMay 22, 2020 09:56
Co-authored-by: Rémi Lapeyre <remi.lapeyre@henki.fr>
Co-authored-by: Rémi Lapeyre <remi.lapeyre@henki.fr>
@uriyyo
Copy link
MemberAuthor

@remilapeyre I agree with your suggestions, thanks.

@uriyyouriyyo changed the titlebpo-29847: Fix bug when Path takes and ignores **kwargsbpo-29847: Fix bug when PurePath takes and ignores **kwargsMay 22, 2020
@uriyyouriyyo changed the titlebpo-29847: Fix bug when PurePath takes and ignores **kwargsbpo-29847: Fix bug when Path takes and ignores **kwargsMay 22, 2020
@netlify
Copy link

netlifybot commentedDec 6, 2022
edited
Loading

Deploy Preview forpython-cpython-preview canceled.

NameLink
🔨 Latest commit8ef2e9e
🔍 Latest deploy loghttps://app.netlify.com/sites/python-cpython-preview/deploys/638fc5db834885000837776c

@brettcannonbrettcannon changed the titlebpo-29847: Fix bug when Path takes and ignores **kwargsgh-74033: Fix bug when Path takes and ignores **kwargsDec 16, 2022
Copy link
Member

@brettcannonbrettcannon left a 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.

@bedevere-bot
Copy link

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 phraseI 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.

@uriyyo
Copy link
MemberAuthor

I have made the requested changes; please review again

@bedevere-bot
Copy link

Thanks for making the requested changes!

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

Copy link
Member

@brettcannonbrettcannon left a 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.

@bedevere-bot
Copy link

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 phraseI 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.

@uriyyo
Copy link
MemberAuthor

I have made the requested changes; please review again.

@bedevere-bot
Copy link

Thanks for making the requested changes!

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

@miss-islington
Copy link
Contributor

Status check is done, and it's a success ✅.

@miss-islingtonmiss-islington merged commit080cb27 intopython:mainJan 14, 2023
@brettcannon
Copy link
Member

Thanks for the PR and the patience!

uriyyo reacted with hooray emojiuriyyo reacted with heart emoji

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

Reviewers

@brettcannonbrettcannonbrettcannon approved these changes

@barneygalebarneygaleAwaiting requested review from barneygale

+1 more reviewer

@remilapeyreremilapeyreremilapeyre approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@uriyyo@bedevere-bot@miss-islington@brettcannon@remilapeyre@hauntsaninja@the-knights-who-say-ni@ezio-melotti@iritkatriel

[8]ページ先頭

©2009-2025 Movatter.jp