bpo-33217: Raise TypeError for non-Enum lookups in Enums#6651
bpo-33217: Raise TypeError for non-Enum lookups in Enums#6651ethanfurman merged 4 commits intopython:masterfrom
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 our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. When your account is ready, please add a comment in this pull request Thanks again to your contribution and we look forward to looking at it! |
|
Looks good! Let me know when you've signed the CLA so I can double-check the automated system for acknowleding that. |
|
Hello @ethanfurman! Thank You for the quick review! :-) I've already signed the CLA - here is the agreement: https://secure.na1.echosign.com/public/viewAgreement?tsid=CBFCIBAA3AAABLblqZhAVAlSqtWnup3E3C9Qf0EseoRcmZj4yhFrpobfhz-VZH-xgCtqFOTtIkHc4yW9EquEd7U7tJ7txdGUSWJtLIg_B& |
| if not isinstance(member, Enum): | ||
| raise TypeError( | ||
| "Unsupported operands type(s) for 'in': '%s' and '%s'" % ( | ||
| type(member).__qualname__, type(Enum).__qualname__)) |
There was a problem hiding this comment.
I just noticed that if we use cls.__qualname__ instead of type(Enum).__qualname__, we get a much better output.
For example if we have the following program
ABC = enum.Flag('ABC', 'a, b, c')
'test' in ABCthen after changing to cls.__qualname__, we get:
TypeError: Unsupported operands type(s) for 'in': 'str' and 'ABC'
which looks better than what we were getting before:
TypeError: Unsupported operands type(s) for 'in': 'str' and 'EnumMeta'
Which one should we use?
There was a problem hiding this comment.
That definitely looks better; go ahead and use cls.__qualname__ where ever it's needed.
| return NotImplemented | ||
| raise TypeError( | ||
| "Unsupported operands type(s) for 'in': '%s' and '%s'" % ( | ||
| type(other).__qualname__, type(self).__qualname__)) |
There was a problem hiding this comment.
We would need similar changes here as well.
|
I have made the requested changes @ethanfurman. |
|
Oops! The tests are failing - Looks like not all the classes have a |
| raise TypeError( | ||
| "Unsupported operands type(s) for 'in': '%s' and '%s'" % ( | ||
| type(other).__qualname__, type(self).__qualname__)) | ||
| type(other).__qualname__, self.__qualname__)) |
There was a problem hiding this comment.
self is the member itself, not the class -- that is why the __qualname__ lookup is faling; use self.__class__._qualname__ instead.
|
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. |
|
Hey @ethanfurman, I don't want to sound desperate but I am rather too excited. Can I have one final review, please? 😋 |
| 3 in Season | ||
| with self.assertRaises(TypeError): | ||
| 'AUTUMN' in Season | ||
|
|
There was a problem hiding this comment.
Let's change these to assertRaisesRegex and test for the qualname being returned; something like
self.assertRaisesRegex(TypeError, r"unsupported operand type.s. for .in.: .int. and .Season."):
|
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 |
Raise :exc:
TypeErrorwhen attempting to lookup non-Enum objects in a :class:Enummember (e.g.1 in Color) or non-Flag objects in a :class:Flagmember (e.g.1 in Perm.RW).https://bugs.python.org/issue33217