Skip to content
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: modify __repr__, __str__; update docs #84247

Closed
ethanfurman opened this issue Mar 25, 2020 · 25 comments
Closed

Enum: modify __repr__, __str__; update docs #84247

ethanfurman opened this issue Mar 25, 2020 · 25 comments
Assignees
Labels
3.10 only security fixes 3.11 bug and security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@ethanfurman
Copy link
Member

BPO 40066
Nosy @warsaw, @rhettinger, @scoder, @vstinner, @tiran, @ezio-melotti, @ethanfurman, @serhiy-storchaka, @The-Compiler, @vedgar, @mscuthbert, @pablogsal, @kulikjak, @kumaraditya303
PRs
  • bpo-40066: Enum: modify repr() and str() #22392
  • bpo-40066: Enum global repr doctest #25116
  • bpo-40066: Enum: add (re)import of Flag for doctests #25118
  • bpo-40066: [Enum] update str() and format() output #30582
  • Revert "bpo-40066: [Enum] update str() and format() output" #30632
  • bpo-40066: Revert "Revert "bpo-40066: [Enum] update str() and format() output..." #30637
  • bpo-40066: [Enum] fix doc and unit tests #30643
  • 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:

    assignee = 'https://github.com/ethanfurman'
    closed_at = <Date 2022-01-23.00:28:03.406>
    created_at = <Date 2020-03-25.19:48:58.219>
    labels = ['type-feature', 'library', '3.10', '3.11']
    title = 'Enum: modify __repr__, __str__; update docs'
    updated_at = <Date 2022-01-23.00:28:03.406>
    user = 'https://github.com/ethanfurman'

    bugs.python.org fields:

    activity = <Date 2022-01-23.00:28:03.406>
    actor = 'ethan.furman'
    assignee = 'ethan.furman'
    closed = True
    closed_date = <Date 2022-01-23.00:28:03.406>
    closer = 'ethan.furman'
    components = ['Library (Lib)']
    creation = <Date 2020-03-25.19:48:58.219>
    creator = 'ethan.furman'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 40066
    keywords = ['patch']
    message_count = 25.0
    messages = ['365019', '366190', '376900', '376913', '376915', '376918', '377495', '378019', '378239', '389869', '390895', '390956', '390976', '396790', '410676', '410711', '410758', '410773', '410775', '410796', '410797', '410801', '410802', '410803', '410805']
    nosy_count = 16.0
    nosy_names = ['barry', 'rhettinger', 'scoder', 'vstinner', 'christian.heimes', 'ezio.melotti', 'mrabarnett', 'eli.bendersky', 'ethan.furman', 'serhiy.storchaka', 'The Compiler', 'veky', 'mscuthbert', 'pablogsal', 'kulikjak', 'kumaraditya']
    pr_nums = ['22392', '25116', '25118', '30582', '30632', '30637', '30643']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue40066'
    versions = ['Python 3.10', 'Python 3.11']

    @ethanfurman
    Copy link
    Member Author

    Serhiy had the idea of having Enum._convert also modify the __str__ and __repr__ of newly created enumerations to display the module name instead of the enumeration name (https://bugs.python.org/msg325007):

    --> socket.AF_UNIX
    <AddressFamily.AF_UNIX: 1> ==> <socket.AF_UNIX: 1>

    --> print(socket.AF_UNIX)
    AddressFamily.AF_UNIX ==> socket.AF_UNIX

    Thoughts?

    @ethanfurman ethanfurman added the 3.9 only security fixes label Mar 25, 2020
    @ethanfurman ethanfurman self-assigned this Mar 25, 2020
    @ethanfurman ethanfurman added type-feature A feature request or enhancement 3.9 only security fixes labels Mar 25, 2020
    @ethanfurman ethanfurman self-assigned this Mar 25, 2020
    @ethanfurman ethanfurman added the type-feature A feature request or enhancement label Mar 25, 2020
    @vedgar
    Copy link
    Mannequin

    vedgar mannequin commented Apr 11, 2020

    _in some cases when enum instances are exposed as module globals_

    Yes. And repr should be inverse of eval, but it's probably too late for that. :-/

    @ethanfurman
    Copy link
    Member Author

    Looks like the re module's flags have been updated separately in bpo-36548:

      >>> import re
      >>> re.I
      re.IGNORECASE
    
      >>> print(re.I)
      # should also be re.IGNORECASE
    
      >>> re.I|re.S|re.X
      re.IGNORECASE|re.DOTALL|re.VERBOSE

    For stdlib Enum conversions are we happy with that? Or should __str__ just print the numeric value?

    @ethanfurman ethanfurman added 3.10 only security fixes and removed 3.9 only security fixes labels Sep 14, 2020
    @vedgar
    Copy link
    Mannequin

    vedgar mannequin commented Sep 14, 2020

    If it's considered to be not too backwards-incompatible, I think it would be nice to have str different from repr. That way we can finetune what exactly we need. But we can already do almost exactly that with *int* instead of *str*, so it's not too compelling.

    Much more important thing is the "repr as inverse of eval". Is there any way we can have that for our own enums (as a mixin or a decorator)?

        @module_global(re)
        class RegexFlag(Enum):
            ...

    It would be fantastic. :-)

    @ethanfurman
    Copy link
    Member Author

    "repr as inverse of eval" is nice to have, but it is not a requirement.

    @vedgar
    Copy link
    Mannequin

    vedgar mannequin commented Sep 15, 2020

    Noone said it is a requirement, I just said it would be nice to have it factored out as a decorator or something instead of having to write __repr__ over and over again.

    @ethanfurman
    Copy link
    Member Author

    At this point, the PR has made the following changes:

    • normal Enums

      • repr() -> "classname.membername"
      • str() -> "membername"
    • stdlib Enums available as module attributes (RegexFlag, AddressFamily, etc.)

      • repr() -> "modulename.membername"
      • str() -> "membername"

    @ethanfurman
    Copy link
    Member Author

    Python-Dev thread [0], summary below:

    As you may have noticed, Enums are starting to pop up all
    over the stdlib [1].

    To facilitate transforming existing module constants to
    IntEnums there is IntEnum._convert_. In bpo-36548 [2]
    Serhiy modified the repr of RegexFlag:

    import re
    re.I
    re.IGNORECASE

    I think for converted constants that that looks nice.
    For anyone that wants the actual value, it is of course
    available as the .value attribute:

    re.I.value
    2

    I'm looking for arguments relating to:

    • should convert make the default repr be
      module_name.member_name?

    • should convert make the default str be the same,
      or be the numeric value?

    After discussions with Guido I made a (largely done) PR [3] which:

    for stdlib global constants (such as RE)

    • repr() -> uses module.member_name
    • str() -> uses member_name

    for stdlib non-global constants, and enums in general

    • repr() -> uses class.member_name
    • str() -> uses member_name

    The questions I would most appreciate an answer to at this point:

    • do you think the change has merit?
    • why /shouldn't/ we make the change?

    As a reminder, the underlying issue is trying to keep at least the stdlib Enum representations the same for those that are replacing preexisting constants.

    [0] https://mail.python.org/archives/list/python-dev@python.org/message/CHQW6THTDYNPPFWQ2KDDTUYSAJDCZFNP/

    [1] I'm working on making their creation faster. If anyone wanted to convert EnumMeta to C I would be grateful.

    [2] https://bugs.python.org/issue36548

    [3] #22392

    @vedgar
    Copy link
    Mannequin

    vedgar mannequin commented Oct 8, 2020

    • do you think the change has merit?

    Absolutely.

    • why /shouldn't/ we make the change?

    Well, standard backward compatibility stuff. :-)

    @ethanfurman ethanfurman added the stdlib Python modules in the Lib dir label Mar 26, 2021
    @ethanfurman ethanfurman changed the title Enum._convert should change __repr__ and/or __str__ to use module name instead of class name Enum: modify __repr__, __str__; update docs Mar 26, 2021
    @ethanfurman
    Copy link
    Member Author

    New changeset b775106 by Ethan Furman in branch 'master':
    bpo-40066: Enum: modify repr() and str() (GH-22392)
    b775106

    @scoder
    Copy link
    Contributor

    scoder commented Apr 12, 2021

    why /shouldn't/ we make the change?

    It breaks doctests, and probably some other unit tests, too, e.g. for output formatting.

    What should we suggest users to do? Replace

        >>> get_flag(…)
        <app_enums.TrickyFlag: 1>

    by

        >>> get_flag(…) == app_enums.TrickyFlag  or get_flag(…)  # (also show result on failure)
        True

    and

        assertEqual(
            "You caught the %s flag!" % result,
            "You caught the app_enums.TrickyFlag flag!")

    by

    assertEqual(
        ("You caught the %s flag!" % result).replace("app_enums.", ""),
        "You caught the TrickyFlag flag!")
    

    ?

    Note that using "%r" does not help, since it's also backwards incompatible.

    For their own enums, users can probably backport (or forward-port) "__str__()" and "__repr__()" themselves in order to work around this difference. But it's something they would have to do.

    I certainly understand the reasoning, and it also makes new Py3.10-only doctests nicer, actually, but IMHO it counts as deliberate breakage.

    @The-Compiler
    Copy link
    Mannequin

    The-Compiler mannequin commented Apr 13, 2021

    I'm probably a bit late to the party as well, but as some additional datapoints:

    @ethanfurman
    Copy link
    Member Author

    Thank you for the feedback.

    The new str() and repr() make more sense for Flag-based enumerations, and I'm hesitant to have different formats for Enum- vs Flag-based enums.

    Would it be helpful to have another base Enum class to derive from that maintained the original str() and repr() formats?

    @ethanfurman ethanfurman reopened this Apr 13, 2021
    @mscuthbert
    Copy link
    Mannequin

    mscuthbert mannequin commented Jun 30, 2021

    It may be helpful for the enum module to come with transitional functions like "pre310_str()" "pre310_repr()" "pre310_flag_str()" etc. so that people who are writing doctests that need to function on both < 3.10 and 3.10+ can temporarily do a "Enum.__str__ = pre310_str" in their test suites (and of course restore it later) until <=3.9 is no longer supported. Otherwise everyone with doctest suites will be doing this ourselves.

    @ethanfurman
    Copy link
    Member Author

    New changeset acf7403 by Ethan Furman in branch 'main':
    bpo-40066: [Enum] update str() and format() output (GH-30582)
    acf7403

    @tiran
    Copy link
    Member

    tiran commented Jan 16, 2022

    #74767 broke doc tests in 3.11 branch:

    File "library/enum.rst", line ?, in default
    Failed example:
        Color(0)
    Exception raised:
        Traceback (most recent call last):
          File "/home/runner/work/cpython/cpython/Lib/doctest.py", line 1346, in __run
            exec(compile(example.source, filename, "single",
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
          File "<doctest default[0]>", line 1, in <module>
            Color(0)
            ^^^^^
        NameError: name 'Color' is not defined

    @tiran tiran added 3.11 bug and security fixes release-blocker labels Jan 16, 2022
    @kulikjak
    Copy link
    Mannequin

    kulikjak mannequin commented Jan 17, 2022

    This also broke our Solaris build with the following error:

    ======================================================================
    FAIL: testGetaddrinfo (test.test_socket.GeneralModuleTests)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/..../cpython-main/Lib/test/test_socket.py", line 1523, in testGetaddrinfo
        self.assertEqual(repr(type), '<SocketKind.SOCK_STREAM: 1>')
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    AssertionError: '<SocketKind.SOCK_STREAM: 2>' != '<SocketKind.SOCK_STREAM: 1>'
    - <SocketKind.SOCK_STREAM: 2>
    ?                          ^
    + <SocketKind.SOCK_STREAM: 1>
    ?                          ^

    (test.test_socket.GeneralModuleTests fails with the same error).

    The issue is almost certainly that on Solaris, SOCK_STREAM is defined as 2 rather than 1; the following simple program confirms that:

    #include <stdio.h>
    #include <sys/socket.h>
    
    void main() {
    	printf("%d\n", SOCK_STREAM);
    }

    I'm just not sure whether to fix this with assertRegex or a special branch for Solaris (though I am not sure whether everybody else uses 1 or it's more varied).

    @vstinner
    Copy link
    Member

    New changeset 42a64c0 by Victor Stinner in branch 'main':
    Revert "bpo-40066: [Enum] update str() and format() output (GH-30582)" (GH-30632)
    42a64c0

    @vstinner
    Copy link
    Member

    Sorry, I had to revert the change since it broke the CI and it prevented to merge new PRs. Tell me if I can help to get this test fixed and to get this change merged again.

    By the way, the PR 30582 was merged even if the Docs CI failed.

    @ethanfurman
    Copy link
    Member Author

    After merging in doc fix by kumaraditya303, I'll update tests so Solaris passes.

    @ethanfurman
    Copy link
    Member Author

    New changeset 83d544b by Kumar Aditya in branch 'main':
    bpo-40066: [Enum] skip failing doc test (GH-30637)
    83d544b

    @vstinner
    Copy link
    Member

    self.assertEqual(repr(type), '<SocketKind.SOCK_STREAM: 1>')

    For this one, I suggest to replace the value with "..." doctest pattern.

    @ethanfurman
    Copy link
    Member Author

    vstinner wrote:
    --------------

    > self.assertEqual(repr(type), '<SocketKind.SOCK_STREAM: 1>')

    For this one, I suggest to replace the value with "..." doctest pattern.

    That bit of code is from the unittest suite, not the doctest suite.

    I went with:

    self.assertEqual(repr(type), '<SocketKind.SOCK_STREAM: %r>' % type.value)
    

    @vstinner
    Copy link
    Member

    I created python/core-workflow#424 "Should we make the Docs CI mandatory on the Python main branch?".

    @ethanfurman
    Copy link
    Member Author

    New changeset 62a6594 by Ethan Furman in branch 'main':
    bpo-40066: [Enum] fix tests (GH-30643)
    62a6594

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    bdegreve pushed a commit to cocamware/lass that referenced this issue Nov 20, 2022
    Python 3.11 has changed behavior of __str__ for IntEnum. Before it would
    return the name, like "Color.RED", now it's equivalent to
    str(self.value), for example "1".__repr__ is unchanged [1].
    
    Python 3.11 also introduced StrEnum, which we can use instead of mixing
    in str type for our StrEnumDefinition, and that introcudes the same
    change of __str__ behavior: it returns the value as string.
    
    The corresponds, in spirit, to what I often did in Python:
    
        class Color(Enum):
            RED="red"
            BLUE="blue"
            def __str__(self):
                return self.value
    
    To ensure consistent behavior for different Python versions, and
    because we can determine our own rules for the enum types we define, we
    mimic the 3.11 __str__ behavior on IntEnumDefinition and
    StrEnumDefinition.
    
    We use the same trick, by setting the __str__ property to the version of
    the underlying type: str.__str__ and int.__repr__.  int.__str__ won't do
    because int directly inherits object.__str__ that basically repr(self),
    and thus back to Enum.__repr__. So we need int.__repr__ to return the
    integer as string.
    
    [1] python/cpython#84247
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes 3.11 bug and security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants