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
enum error messages should use __qualname__ #78624
Comments
The __repr__ output of an enum class should use __qualname__ instead of __name__. The following example shows the problem: import enum
class X:
class I:
pass
class Y:
class I(enum.Enum):
pass
print(X.I)
print(Y.I) This prints: <class '__main__.X.I'> I would have expected it to print <class '__main__.X.I'> or even for maximum consistency <class '__main__.X.I'> |
__qualname__ should be used only together with __module__. I agree that the repr of enum should be more consistent with the repr of class. |
Hi, |
Serhiy, would it be ok to put '__module__' + '.' + __qualname__ here? |
Serhiy, why should __qualname__ always be used together with __module__? I can't seem to find a valid reason, I've been through the pep. <made a PR due to inactivity> |
I've created a separate PR that also changes the __str__s. |
After some thinking I've come to the conclusion that making the __str__s use the fully qualified name was a bad idea. I've closed my PR. |
Because what is the purpose of using __qualname__? |
I think using more longer name in repr and/or str for *instances* of enum classes is not good idea. They are already verbose, and this will make them more verbose. Actually in some cases when enum instances are exposed as module globals, I would want to make them including the module name instead of the enum class name. For example: >>> import socket
>>> socket.AF_UNIX
<AddressFamily.AF_UNIX: 1>
>>> print(socket.AF_UNIX)
AddressFamily.AF_UNIX "socket.AF_UNIX" instead of "AddressFamily.AF_UNIX" would look better to me. |
Serhiy said:
I'm okay with verbose reprs, as debugging is the primary feature for those (at least for me). I'm not okay with __str__ being other than what it is now (but see below). Serhiy also said:
Since AddressFamily, and other stdlib converted constants, are created user |
Okay, I might be changing my mind. In most cases I suspect the difference would be minimal, but when it isn't, it really isn't. Take this example from a pydoc test: class Color(enum.Enum)
| Color(value, names=None, *, module=None, qualname=None, type=None, start=1)
|
| An enumeration.
|
| Method resolution order:
| Color
| enum.Enum
| builtins.object
|
| Data and other attributes defined here:
|
- | blue = <test.test_enum.TestStdLib.Color.blue: 3>
+ | blue = <Color.blue: 3>
|
- | green = <test.test_enum.TestStdLib.Color.green: 2>
+ | green = <Color.green: 2>
|
- | red = <test.test_enum.TestStdLib.Color.red: 1>
+ | red = <Color.red: 1> It feels like the important information is completely lost in the noise. Okay, I'm rejecting the __repr__ changes. Besides the potential verbosity, there should usually only be one of any particular Enum, __module__ and __qualname__ are both readily available when there are more than one (either on accident or by design), and users can modify their own __repr__s if they like. I'm still thinking about the change in _convert_ to modify __str__ to use the module name instead of the class name.... Here are my questions about that:
|
Can we at least get the __qualname__ in exception messages? Currently enum.Enum.__new__() and enum.Enum._missing_() use: raise ValueError("%r is not a valid %s" % (value, cls.__name__)) IMHO this should be: raise ValueError("%r is not a valid %s" % (value, cls.__qualname__)) in both spots. Example code: class Person:
class Type(enum.Enum):
EMPLOYEE = "employee"
CUSTOMER = "customer"
SUPPLIER = "supplier"
class Contact:
class Type(enum.Enum):
EMAIL = "email"
PHONE = "phone"
MOBILE = "mobile"
with this the following code:
Person.Type('foo') raises the exception:
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/local/Cellar/python/3.7.4/Frameworks/Python.framework/Versions/3.7/lib/python3.7/enum.py", line 310, in __call__
return cls.__new__(cls, value)
File "/usr/local/Cellar/python/3.7.4/Frameworks/Python.framework/Versions/3.7/lib/python3.7/enum.py", line 564, in __new__
raise exc
File "/usr/local/Cellar/python/3.7.4/Frameworks/Python.framework/Versions/3.7/lib/python3.7/enum.py", line 548, in __new__
result = cls._missing_(value)
File "/usr/local/Cellar/python/3.7.4/Frameworks/Python.framework/Versions/3.7/lib/python3.7/enum.py", line 577, in _missing_
raise ValueError("%r is not a valid %s" % (value, cls.__name__))
ValueError: 'foo' is not a valid Type IMHO the exception message:
would be much more helpful and unambiguous. And BTW, maybe we should suppress exception chaining here, i.e. use: raise ValueError("%r is not a valid %s" % (value, cls.__qualname__)) from None |
Both are good suggestions. The first should definitely happen. I'll investigate the second. |
If someone would like to make a PR for using __qualname__ in error messages that would be great. :) |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: