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 equality across modules: comparing objects instead of values #74730

Closed
MadhavDatt mannequin opened this issue Jun 2, 2017 · 14 comments
Closed

Enum equality across modules: comparing objects instead of values #74730

MadhavDatt mannequin opened this issue Jun 2, 2017 · 14 comments
Assignees
Labels
docs Documentation in the Doc dir

Comments

@MadhavDatt
Copy link
Mannequin

MadhavDatt mannequin commented Jun 2, 2017

BPO 30545
Nosy @bitdancer, @ethanfurman
Files
  • enum_bug-flawless_example.zip: Structurally similar example program running flawlessly
  • enum_reload_example.zip
  • 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 2017-06-08.18:01:08.648>
    created_at = <Date 2017-06-02.04:58:59.769>
    labels = ['3.7', 'invalid', 'type-bug', 'library']
    title = 'Enum equality across modules: comparing objects instead of values'
    updated_at = <Date 2018-11-16.13:05:55.653>
    user = 'https://bugs.python.org/MadhavDatt'

    bugs.python.org fields:

    activity = <Date 2018-11-16.13:05:55.653>
    actor = 'Sebastian H\xc3\xb6fer'
    assignee = 'ethan.furman'
    closed = True
    closed_date = <Date 2017-06-08.18:01:08.648>
    closer = 'ethan.furman'
    components = ['Library (Lib)']
    creation = <Date 2017-06-02.04:58:59.769>
    creator = 'Madhav Datt'
    dependencies = []
    files = ['47747', '47936']
    hgrepos = []
    issue_num = 30545
    keywords = []
    message_count = 11.0
    messages = ['294983', '295025', '295032', '295036', '295038', '295457', '323411', '323418', '323432', '323433', '329994']
    nosy_count = 6.0
    nosy_names = ['r.david.murray', 'ethan.furman', 'adrianwan2', 'Madhav Datt', 'Markus Wegmann', 'Sebastian H\xc3\xb6fer']
    pr_nums = []
    priority = 'normal'
    resolution = 'not a bug'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue30545'
    versions = ['Python 3.7']

    @MadhavDatt
    Copy link
    Mannequin Author

    MadhavDatt mannequin commented Jun 2, 2017

    The problem is described with an example in this StackOverflow question (https://stackoverflow.com/questions/26589805/python-enums-across-modules). Like in C and other languages, I would expect Enum equality to work across modules and not compare enum states/values, instead of just checking for the same object.

    A possible simple fix for this problem would be to override the __eq__() function by default in the enum.Enum class with the following:

    def __eq__(self, other):
        if isinstance(other, self.__class__):
            return self.value == other.value
        return False

    I would be happy to create a GitHub pull request to fix this, however, I do not have the experience or knowledge to know if

    • the current behavior is by design;
    • whether this is worth fixing; and
    • whether fixing this will break anything else.

    @MadhavDatt MadhavDatt mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Jun 2, 2017
    @ethanfurman
    Copy link
    Member

    Two points:

    • Python 2.7 was the version marked, but 2.7 does not come with Enum
      (wasn't introduced until 3.4 -- the third-party backport does work
      on 2.7)

    • the problem in the SO question is not caused by Enum, but by
      re-importing a module under a different name which results in two
      different Enum classes that happen to look identical, but are not --
      so the change you propose would not help; also, since Enum members
      with the same value are mapped to the same member your change does not
      provide any new behavior.

    So, in summary, the bug here is in the user's code.

    @ethanfurman ethanfurman added the 3.7 (EOL) end of life label Jun 2, 2017
    @MadhavDatt
    Copy link
    Mannequin Author

    MadhavDatt mannequin commented Jun 2, 2017

    Thanks a lot for those points Ethan. I feel I haven't done a very good job of explaining the bug, but let me use an example. Let's say we have an Enum called MyEnum, which is in a Python module called ModuleA. ModuleB imports ModuleA, and ModuleC imports both, ModuleA and ModuleB. Now, in ModuleC, I have ModuleB.some_function() return a MyEnum state, which I pass as a parameter to ModuleA.other_function() where it is compared to MyEnum states. Here the comparison fails even though it should not have. Obviously, this problem would not arise without such imports, and so is pretty specific, but I hope this makes explains it a little better.

    @MadhavDatt MadhavDatt mannequin reopened this Jun 2, 2017
    @bitdancer
    Copy link
    Member

    Can you provide actual code that demonstrates the issue you are talking about?

    @ethanfurman
    Copy link
    Member

    If your example code is the same as the code in the SO problem, then my previous points stand.

    According to the plain-English description you provided the comparison would succeed, so if you have example code which:

    • doesn't involve ModuleA being the __main__ script module, and
    • has the comparison fail, then

    please share it. ;) (As a comment/message here is fine.)

    @ethanfurman
    Copy link
    Member

    No test code has been provided, so lacking any evidence of this problem I am closing this issue.

    Do not reopen without testable code to show the failure.

    @MarkusWegmann
    Copy link
    Mannequin

    MarkusWegmann mannequin commented Aug 11, 2018

    Hi there!

    I came as well in contact with this kind of bug. Sadly, I could not replicate it with a simplified toy example.

    You can experience the bug, if checkout

    https://github.com/Atokulus/flora_tools/tree/56bb17ea33c910915875214e916ab73f567b3b0c

    and run

    python3.7 main.py generate_code -d ..

    You find the crucial part where the identity check fails at 'radio_math.py:102' in function get_message_toa. The program then fails, due to the wrong program flow.

    `
    C:\Users\marku\AppData\Local\Programs\Python\Python37\python.exe C:/Users/marku/PycharmProjects/flora_tools/flora_tools/__main__.py generate_code -d C:\Users\marku\Documents\flora
    Traceback (most recent call last):
      File "C:/Users/marku/PycharmProjects/flora_tools/flora_tools/__main__.py", line 110, in <module>
        main()
      File "C:/Users/marku/PycharmProjects/flora_tools/flora_tools/__main__.py", line 104, in main
        generate_code(args.path)
      File "C:/Users/marku/PycharmProjects/flora_tools/flora_tools/__main__.py", line 58, in generate_code
        code_gen = CodeGen(flora_path)
      File "C:\Users\marku\PycharmProjects\flora_tools\flora_tools\codegen\codegen.py", line 37, in __init__
        self.generate_all()
      File "C:\Users\marku\PycharmProjects\flora_tools\flora_tools\codegen\codegen.py", line 40, in generate_all
        self.generate_radio_constants()
      File "C:\Users\marku\PycharmProjects\flora_tools\flora_tools\codegen\codegen.py", line 72, in generate_radio_constants
        radio_toas.append([math.get_message_toa(payload) for payload in payloads])
      File "C:\Users\marku\PycharmProjects\flora_tools\flora_tools\codegen\codegen.py", line 72, in <listcomp>
        radio_toas.append([math.get_message_toa(payload) for payload in payloads])
      File "C:\Users\marku\PycharmProjects\flora_tools\flora_tools\radio_math.py", line 130, in get_message_toa
        ) / self.configuration.bitrate
    TypeError: unsupported operand type(s) for +: 'int' and 'NoneType'

    Process finished with exit code 1
    `

    In the appendix, you find a toy project with similar structure, which in contrast to flora_tools works flawlessly.

    I hope there is somebody out there with a little more experience in spotting the cause. If there is a bug in Python or CPython, it might have quite a security & safety impact.

    Meanwhile I will hotfix my program by comparing the unerlying enum values.

    Best regards
    Atokulus

    @ethanfurman
    Copy link
    Member

    Markus Wegmann said:
    --------------------

    In the appendix, you find a toy project with similar structure, which > in contrast to flora_tools works flawlessly.

    I appreciate your attempt to reproduce the problem, but since you weren't able to, the issue is probably somewhere else and not in Enum.

    TypeError: unsupported operand type(s) for +: 'int' and 'NoneType'

    Your Enum example in flawless is not an IntEnum, so the error (unable to add an integer to None) seems entirely unrelated.

    @MarkusWegmann
    Copy link
    Mannequin

    MarkusWegmann mannequin commented Aug 12, 2018

    Hi Ethan

    Your Enum example in flawless is not an IntEnum, so the error (unable to add an integer to None) seems entirely unrelated.

    The TypeError is just a consequence of the faulty Enum identity comparison some lines before. I mentioned the TypeError so you can verify whether your Python version takes the same program flow.

    I also did further research. The Enum's are definitely different regarding the module path -- the instance comparison will therefore return False. I checked __module__ + __qualname__:

    flora_tools.radio_configuration.RadioModem.LORA

    vs.

    radio_configuration.RadioModem.LORA

    The cause is the wrong import statement in flora_tools/codegen/codegen.py:

    from radio_configuration import RadioConfiguration,\ RADIO_CONFIGURATIONS

    It should have been

    from flora_tools.radio_configuration import RadioConfiguration\ RADIO_CONFIGURATIONS

    The real deal here is why I was allowed to directly import from radio_configuration in the first place. I'm not allowed to directly import a submodule in the toy project without the proper root module name appended. Maybe I don't see the big picture, or have some crude options/monkey_patching enabled.

    Nevertheless, the behaviour regarding Enum comparisons and different import paths seems to me quite misleading.

    Best regards
    Atokulus

    @ethanfurman
    Copy link
    Member

    What you have discovered is not Enum specific, but in fact can happen with any module (as stated in my first comment in this bug report).

    Maybe these SO question will help:

    https://stackoverflow.com/q/2489601/208880

    https://stackoverflow.com/a/4798648/208880

    @SebastianHfer
    Copy link
    Mannequin

    SebastianHfer mannequin commented Nov 16, 2018

    I ran into the same issue and while I see that this is not a bug I would suggest that this behaviour at least deserves a warning in the documentation under
    https://docs.python.org/3/library/enum.html#comparisons

    This unexpected behaviour can not only occur with messed up imports, but also when a module uses the importlib.reload() function and submodules are reloaded during runtime.

    I attached a minimal working example.
    In our case we have a central module where all constants, dataclasses and enums are defined. A main program imports several submodules which all import these datatypes. Reloading a submodule results in new object ids for the enums in this submodule and comparing data between different modules suddenly results in inconsistent behaviour.

    It took a while to debug, because except for the object id the enums look exactly the same and before reading this thread I did not realize that the comparison is actually done by object ids.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @ForrestTrepte
    Copy link

    ForrestTrepte commented Apr 24, 2022

    I encountered this problem when using the autoreload extension. It was confusing when comparing enum values sometimes failed when my enum class was in a module that was reloaded. I used the eq function at the top of this issue to work around the problem.

    @ethanfurman
    Copy link
    Member

    Okay, a warning in the docs emphasizing that regular enums compare by id (as opposed to IntEnum, StrEnum, etc) and reloading modules or importing modules under different names will cause that to fail, seems appropriate.

    @ethanfurman ethanfurman reopened this Apr 25, 2022
    @AlexWaygood AlexWaygood added docs Documentation in the Doc dir and removed invalid stdlib Python modules in the Lib dir 3.7 (EOL) end of life type-bug An unexpected behavior, bug, or error labels May 4, 2022
    ethanfurman added a commit that referenced this issue Apr 3, 2023
    fix FlagBoundary statements
    add warning about reloading modules and enum identity
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Apr 3, 2023
    fix FlagBoundary statements
    add warning about reloading modules and enum identity
    (cherry picked from commit 5ffc1e5)
    
    Co-authored-by: Ethan Furman <ethan@stoneleaf.us>
    miss-islington added a commit that referenced this issue Apr 3, 2023
    fix FlagBoundary statements
    add warning about reloading modules and enum identity
    (cherry picked from commit 5ffc1e5)
    
    Co-authored-by: Ethan Furman <ethan@stoneleaf.us>
    gaogaotiantian pushed a commit to gaogaotiantian/cpython that referenced this issue Apr 8, 2023
    fix FlagBoundary statements
    add warning about reloading modules and enum identity
    warsaw pushed a commit to warsaw/cpython that referenced this issue Apr 11, 2023
    fix FlagBoundary statements
    add warning about reloading modules and enum identity
    @ffissore
    Copy link

    Hi. I stumbled on this issue once more so coded an example that triggers the weird behaviour whilst NOT using any reloading mechanism

    Pls find the code at this gist: https://gist.github.com/ffissore/e882955f8046d6c35821ba53d2134bdb#file-readme-md

    Also, I was checking the docs and I see no mentions of the different way enums equality work

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    docs Documentation in the Doc dir
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants