[3.7] bpo-34282: Fix Enum._convert shadowing members named _convert #9034
Conversation
Lib/enum.py
Outdated
| # issue34282: Members named _convert should take precedence over | ||
| # the _convert method except if the enum also has a behavior named | ||
| # _convert. In which case, it falls back to the default | ||
| # behavior > member ordering where the behavior takes precedence. |
There was a problem hiding this comment.
There should never be a member and a method with the same name. Besides _convert (which is a bug) are there any other examples?
There was a problem hiding this comment.
This is what I was referring to:
>>> import enum
>>>
>>> class Base(enum.Enum):
... def test(self): ...
...
>>> class Test(Base):
... test = enum.auto()
...
>>> Test.test
<function Base.test at 0x000002090A5AD730>It doesn't seem constricted to just _convert.
There was a problem hiding this comment.
It's the same bug, though. Fixing the shadowing bug will resolve this case as well. Enums are special in many ways, but subclass declarations should still override parent's.
There was a problem hiding this comment.
I feel that's out of scope for this PR. I think it would be better served to close this and create a separate PR (and issue?) for it against the master branch and backport that to 3.7 and 3.6. We should still keep the _convert rename for 3.8 though.
There was a problem hiding this comment.
Another approach would be to add it to #8568 and rewrite the code here but I'm not sure if fixing multiple things in a single PR is good practice. The titles and news entries would also have to change.
There was a problem hiding this comment.
As I said in https://bugs.python.org/msg324415, we'll need a patch for 3.7- to fix the member not showing up, a patch in 3.8 to do a rename and deprecation warning, and a reminder to remove the deprecation and legacy _convert in 3.9.
So use one of these two PRs for one, and the other for the other. :)
There was a problem hiding this comment.
The latter two are fine but if we're going to fix the member not showing up by fixing the shadowing bug shouldn't it also be added to #8568? It affects more than just _convert so having the above code snippet change for 3.7-3.6 and then revert in 3.8 would be strange.
There was a problem hiding this comment.
Yes, the actual fix from the 3.7 patch needs to also be in the 3.8 patch.
|
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. |
|
Thanks for making the requested changes! @ethanfurman: please review the changes made to this pull request. |
|
Looking good. Now we need one or two tests: one for verifying that |
|
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. |
|
Sorry to bother you, but shouldn't this be backported to 3.6? |
|
Thanks @orlnub123 for the PR, and @ethanfurman for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6. |
…nvert (pythonGH-9034) * Fix Enum._convert shadowing members named _convert (cherry picked from commit c0d63bf) Co-authored-by: orlnub123 <orlnub123@gmail.com>
|
GH-9229 is a backport of this pull request to the 3.6 branch. |
|
Not a bother, and yes, thank you. |
This is the 3.7 patch for the issue below.
https://bugs.python.org/issue34282