Issue 34443: enum error messages should use __qualname__

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'>
<enum 'I'>

I would have expected it to print

<class '__main__.X.I'>
<enum 'Y.I'>

or even for maximum consistency

<class '__main__.X.I'>
<enum '__main__.Y.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,
I would like to work on this issue. I can submit a PR by tomorrow (or maybe even later today).

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.

> Serhiy, why should __qualname__ always be used together with __module__?

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 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.

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:
----------------
> 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.

Since AddressFamily, and other stdlib converted constants, are created user `_convert`, I have no problem with that method also changing the __str__ to be `module.member' instead.

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:

- Modify just __str__ or __repr__ as well?
    socket.AF_UNIX instead of AddressFamily.AF_UNIX
    <socket.AddressFamily.AF_UNIX: 1> instead of <AddressFamily.AF_UNIX: 1>

- potential confusion that actual instances of Enum in the stdlib appear
  differently than "regular" Enums?  Or perhaps call out those differences
  in the documentation as examples of customization?

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:

    ValueError: 'foo' is not a valid Type

    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:

    ValueError: 'foo' is not a valid Person.Type

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.  :)

New changeset 323842c2792a81e87917790506ec3457832c84b3 by Ethan Furman (Walter Dörwald) in branch 'master':
bpo-34443: Use __qualname__ instead of __name__ in enum exception messages. (GH-14809)
https://github.com/python/cpython/commit/323842c2792a81e87917790506ec3457832c84b3

pull_requests: + pull_request18526

stage: resolved -> test needed

messages: + msg323801
versions: + Python 3.8