-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
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
bpo-37479: on Enum subclasses with mixins, __format__ uses overridden __str__ #14545
Conversation
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. You can check yourself to see if the CLA has been received. Thanks again for your contribution, we look forward to reviewing it! |
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.
In addition to the test you have, I'd like to see some tests with classes that define format, and with format and str.
Test coverage is now something like:
|
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.
Just a couple changes needed.
Also, please add yourself to the ACKS file.
Thank you for working on this!
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 |
Thanks for the review! I have made the requested changes; please review again :) |
Thanks for making the requested changes! @ethanfurman: 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.
Nearly there!
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 - though check the comment above in case we want to do one more quick round. |
Thanks for making the requested changes! @ethanfurman: please review the changes made to this pull request. |
Sorry @thatneat and @ethanfurman, I had trouble checking out the |
Is that something I can do, or something you'll need to do @ethanfurman?
…On Thu, Jul 4, 2019 at 11:31 AM Miss Islington (bot) < ***@***.***> wrote:
Sorry @thatneat <https://github.com/thatneat> and @ethanfurman
<https://github.com/ethanfurman>, I had trouble checking out the 3.8
backport branch.
Please backport using cherry_picker
<https://pypi.org/project/cherry-picker/> on command line.
cherry_picker 2f19e82 3.8
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#14545?email_source=notifications&email_token=AAZPVU5MNB7SQAPGOS4EFULP5Y6ZTA5CNFSM4H4YFV2KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZH6VPY#issuecomment-508553919>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAZPVU6LWIWBZRJANCZR2D3P5Y6ZTANCNFSM4H4YFV2A>
.
|
@thatneat If you know your way around git and GitHub feel free! My git-fu is very weak. In case you haven't seen it, the dev guide is here: |
It looks like I don't have the permissions I need to use the
Then, manually edit Misc/ACKS to remove the merge markers but keep the additions (search for ">>>>" and delete the 3 lines that don't belong), then
|
… __str__ (pythonGH-14545) * bpo-37479: on Enum subclasses with mixins, __format__ uses overridden __str__
… __str__ (pythonGH-14545) * bpo-37479: on Enum subclasses with mixins, __format__ uses overridden __str__
Thanks @thatneat for the PR, and @ethanfurman for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8. |
Sorry, @thatneat and @ethanfurman, I could not cleanly backport this to |
…ridden __str__ (pythonGH-14545) * bpo-37479: on Enum subclasses with mixins, __format__ uses overridden __str__. (cherry picked from commit 2f19e82) Co-authored-by: thatneat <thatneat@users.noreply.github.com>
GH-22227 is a backport of this pull request to the 3.8 branch. |
Thanks @thatneat for the PR, and @ethanfurman for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7. |
Sorry @thatneat and @ethanfurman, I had trouble checking out the |
…ridden __str__ (pythonGH-14545) * bpo-37479: on Enum subclasses with mixins, __format__ uses overridden __str__. (cherry picked from commit 2f19e82) Co-authored-by: thatneat <thatneat@users.noreply.github.com>
GH-22228 is a backport of this pull request to the 3.7 branch. |
Thanks @thatneat for the PR, and @ethanfurman for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9. |
Sorry, @thatneat and @ethanfurman, I could not cleanly backport this to |
Maybe this change should be mentioned in the 3.11 what's new. It has the potential of breaking a lot of code. |
https://bugs.python.org/issue37479