This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: enum error messages should use __qualname__
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ethan.furman Nosy List: barry, danishprakash, doerwalter, eli.bendersky, enedil, ethan.furman, miss-islington, orlnub123, serhiy.storchaka, underscore_asterisk
Priority: normal Keywords: easy, patch

Created on 2018-08-20 16:34 by doerwalter, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 8989 closed danishprakash, 2018-08-29 09:39
PR 9011 closed orlnub123, 2018-08-30 23:15
PR 14809 merged doerwalter, 2019-07-17 10:23
PR 19166 closed miss-islington, 2020-03-25 19:54
Messages (15)
msg323799 - (view) Author: Walter Dörwald (doerwalter) * (Python committer) Date: 2018-08-20 16:34
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'>
msg323801 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-08-20 17:15
__qualname__ should be used only together with __module__.

I agree that the repr of enum should be more consistent with the repr of class.
msg323822 - (view) Author: (underscore_asterisk) * Date: 2018-08-21 06:27
Hi,
I would like to work on this issue. I can submit a PR by tomorrow (or maybe even later today).
msg324110 - (view) Author: Michał Radwański (enedil) * Date: 2018-08-26 00:50
Serhiy, would it be ok to put '__module__' + '.' + __qualname__ here?
msg324313 - (view) Author: Danish Prakash (danishprakash) * Date: 2018-08-29 09:41
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>
msg324402 - (view) Author: (orlnub123) * Date: 2018-08-30 23:22
I've created a separate PR that also changes the __str__s.
msg324442 - (view) Author: (orlnub123) * Date: 2018-08-31 19:54
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.
msg325006 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-09-11 14:03
> Serhiy, why should __qualname__ always be used together with __module__?

Because what is the purpose of using __qualname__?
msg325007 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-09-11 14:18
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.
msg325020 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2018-09-11 16:00
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.
msg325023 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2018-09-11 16:34
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?
msg348014 - (view) Author: Walter Dörwald (doerwalter) * (Python committer) Date: 2019-07-16 12:03
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
msg348026 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2019-07-16 16:01
Both are good suggestions.  The first should definitely happen.  I'll investigate the second.
msg348048 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2019-07-17 03:11
If someone would like to make a PR for using __qualname__ in error messages that would be great.  :)
msg348123 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2019-07-18 18:37
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
History
Date User Action Args
2022-04-11 14:59:04adminsetgithub: 78624
2020-03-25 19:54:02miss-islingtonsetnosy: + miss-islington

pull_requests: + pull_request18526
2020-03-25 19:51:53ethan.furmansetstatus: open -> closed
stage: patch review -> resolved
resolution: fixed
title: enum repr should use __qualname__ -> enum error messages should use __qualname__
2019-07-18 18:37:17ethan.furmansetmessages: + msg348123
2019-07-17 10:23:16doerwaltersetstage: needs patch -> patch review
pull_requests: + pull_request14603
2019-07-17 03:11:46ethan.furmansetmessages: + msg348048
stage: test needed -> needs patch
2019-07-16 16:01:20ethan.furmansetstatus: closed -> open
resolution: rejected -> (no value)
messages: + msg348026

stage: resolved -> test needed
2019-07-16 12:03:41doerwaltersetmessages: + msg348014
2019-07-16 03:20:42ethan.furmansetstatus: open -> closed
resolution: rejected
stage: patch review -> resolved
2018-09-11 16:34:16ethan.furmansetmessages: + msg325023
2018-09-11 16:00:40ethan.furmansetmessages: + msg325020
2018-09-11 14:18:14serhiy.storchakasetmessages: + msg325007
2018-09-11 14:03:43serhiy.storchakasetmessages: + msg325006
2018-08-31 19:54:40orlnub123setmessages: + msg324442
2018-08-30 23:22:46orlnub123setnosy: + orlnub123
messages: + msg324402
2018-08-30 23:15:27orlnub123setpull_requests: + pull_request8481
2018-08-29 09:41:49danishprakashsetnosy: + danishprakash
messages: + msg324313
2018-08-29 09:39:35danishprakashsetkeywords: + patch
stage: patch review
pull_requests: + pull_request8462
2018-08-26 00:50:14enedilsetnosy: + enedil
messages: + msg324110
2018-08-21 06:27:59underscore_asterisksetnosy: + underscore_asterisk
messages: + msg323822
2018-08-20 17:39:51ethan.furmansetassignee: ethan.furman
2018-08-20 17:15:59serhiy.storchakasetnosy: + barry, eli.bendersky, serhiy.storchaka, ethan.furman

messages: + msg323801
versions: + Python 3.8
2018-08-20 16:35:45doerwaltersetkeywords: + easy
2018-08-20 16:34:28doerwaltercreate