-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add Enum dunder to the list of allowed dunder #10722
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
Conversation
Pierre-Sassoulas
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.
Look great, let's just fix the mypy warning :)
This comment has been minimized.
This comment has been minimized.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #10722 +/- ##
=======================================
Coverage ? 95.97%
=======================================
Files ? 176
Lines ? 19540
Branches ? 0
=======================================
Hits ? 18753
Misses ? 787
Partials ? 0
🚀 New features to boost your workflow:
|
This comment has been minimized.
This comment has been minimized.
Pierre-Sassoulas
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.
What do you think about the new message raised in the primer ? Also could you add a changelog please ? :) See in doc/whatsnew/fragment
|
@Pierre-Sassoulas I added the change log. About the message in the prime, it looks good to me as the message "Bad or misspelled dunder method name generate_next_value." is no longer emitted as the 2 classes inherits from Enum (or StrEnum). |
This comment has been minimized.
This comment has been minimized.
| "__reduce_ex__", | ||
| "__post_init__", # part of `dataclasses` module | ||
| ] | ||
| EXTRA_DUNDER_METHODS: dict[tuple[int, int], list[str]] = { |
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 guess it could be tuple[str] since we have no need to mutate this anywhere in the code; but I don't feel too strongly about changing 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.
hi @mbyrnepr2, I think that'll work too but it'll be tuple[str, str] and we'll need to keep extending it if we add more values to the tuple, so I'm not sure it's worth it as this constant is used very little already.
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.
| EXTRA_DUNDER_METHODS: dict[tuple[int, int], list[str]] = { | |
| EXTRA_DUNDER_METHODS: dict[tuple[int, int], tuple[str, ...]] = { |
Would this work?
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.
yes I think that will work. Thank you for the suggestion.
mbyrnepr2
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 @tqa236 !
Co-authored-by: Mark Byrne <[email protected]>
33ada01
|
🤖 Effect of this PR on checked open source code: 🤖 Effect on ansible:
Effect on django:
This comment was generated for commit 33ada01 |
Co-authored-by: Pierre Sassoulas <[email protected]> Co-authored-by: Mark Byrne <[email protected]> (cherry picked from commit 0426f8b)
…under (#10725) Add Enum dunder to the list of allowed dunder (#10722) (cherry picked from commit 0426f8b) Co-authored-by: Trinh Quoc Anh <[email protected]> Co-authored-by: Pierre Sassoulas <[email protected]> Co-authored-by: Mark Byrne <[email protected]>
Type of Changes
Description
Add a few missing dunder in the Enum class to the allowed list. Also convert
EXTRA_DUNDER_METHODSto a dictionary with Python version.Closes #10435