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

3.11.4: ValueError when inverting enum.Flag member with mask member #105497

Closed
The-Compiler opened this issue Jun 8, 2023 · 14 comments
Closed

3.11.4: ValueError when inverting enum.Flag member with mask member #105497

The-Compiler opened this issue Jun 8, 2023 · 14 comments
Assignees
Labels
3.11 only security fixes 3.12 bugs and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@The-Compiler
Copy link
Contributor

The-Compiler commented Jun 8, 2023

With code such as:

import enum

class Flag(enum.Flag):
    A = 0x01
    B = 0x02
    Mask = 0xff

print(~Flag.A)

Python 3.10.11 prints Flag.B, and so does Python 3.11.3. However, with Python 3.11.4, this happens instead:

Traceback (most recent call last):
  File "/home/florian/tmp/f.py", line 9, in <module>
    print(~Flag.A)
          ^^^^^^^
  File "/usr/lib/python3.11/enum.py", line 1542, in __invert__
    self._inverted_ = self.__class__(self._flag_mask_ ^ self._value_)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/enum.py", line 711, in __call__
    return cls.__new__(cls, value)
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/enum.py", line 1136, in __new__
    raise exc
  File "/usr/lib/python3.11/enum.py", line 1113, in __new__
    result = cls._missing_(value)
             ^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/enum.py", line 1454, in _missing_
    raise ValueError('%r: no members with value %r' % (cls, unknown))
ValueError: <flag 'Flag'>: no members with value 252

As a workaround, a detour via .value works in this case:

>>> Flag((Flag.A | Flag.B).value & ~Flag.A.value)
<Flag.B: 2>

This causes issues with PyQt, which has the following flags (as bindings from C++):

>>> from PyQt6.QtCore import Qt
>>> for e in Qt.KeyboardModifier:
...     print(f"{e.name} = {hex(e.value)}")
... 
NoModifier = 0x0
ShiftModifier = 0x2000000
ControlModifier = 0x4000000
AltModifier = 0x8000000
MetaModifier = 0x10000000
KeypadModifier = 0x20000000
GroupSwitchModifier = 0x40000000
KeyboardModifierMask = 0xfe000000

(Output from Python 3.10 - with Python 3.11, KeyboardModifierMask goes missing in the output, and so does Flag.Mask above, but that seems like a different issue?)

With my project, I'm trying to remove a modifier from the given flags. With Python 3.10.11 and 3.11.3:

>>> (Qt.KeyboardModifier.ShiftModifier | Qt.KeyboardModifier.ControlModifier) & ~Qt.KeyboardModifier.ControlModifier
<KeyboardModifier.ShiftModifier: 33554432>

But with Python 3.11.4, same issue as above:

>>> from PyQt6.QtCore import Qt
>>> (Qt.KeyboardModifier.ShiftModifier | Qt.KeyboardModifier.ControlModifier) & ~Qt.KeyboardModifier.ControlModifier
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.11/enum.py", line 1542, in __invert__
    self._inverted_ = self.__class__(self._flag_mask_ ^ self._value_)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/enum.py", line 711, in __call__
    return cls.__new__(cls, value)
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/enum.py", line 1136, in __new__
    raise exc
  File "/usr/lib/python3.11/enum.py", line 1113, in __new__
    result = cls._missing_(value)
             ^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/enum.py", line 1454, in _missing_
    raise ValueError('%r: no members with value %r' % (cls, unknown))
ValueError: <flag 'KeyboardModifier'>: no members with value 2147483648

As a culprit, I suspect:

cc @ethanfurman @benburrill

Linked PRs

@The-Compiler The-Compiler added the type-bug An unexpected behavior, bug, or error label Jun 8, 2023
@The-Compiler The-Compiler changed the title 3.11.4: ValueError when inverting enum.Flag member with Mask 3.11.4: ValueError when inverting enum.Flag member with mask member Jun 8, 2023
@hugovk hugovk added the stdlib Python modules in the Lib dir label Jun 8, 2023
@Irtiza90 Irtiza90 mentioned this issue Jun 8, 2023
The-Compiler added a commit to qutebrowser/qutebrowser that referenced this issue Jun 8, 2023
Take a detour via .value if on Qt 6, because Python 3.11.4 seems to be
unable to invert enum.Flag values with a mask set.

Fixes #7735

See:
- python/cpython#105497
- https://www.riverbankcomputing.com/pipermail/pyqt/2023-June/045323.html
ethanfurman added a commit that referenced this issue Jun 9, 2023
…H-105542)

When inverting a Flag member (or boundary STRICT), only consider other canonical flags; when inverting an IntFlag member (or boundary KEEP), also consider aliases.
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 9, 2023
…ist. (pythonGH-105542)

When inverting a Flag member (or boundary STRICT), only consider other canonical flags; when inverting an IntFlag member (or boundary KEEP), also consider aliases.
(cherry picked from commit 59f009e)

Co-authored-by: Ethan Furman <ethan@stoneleaf.us>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 9, 2023
…ist. (pythonGH-105542)

When inverting a Flag member (or boundary STRICT), only consider other canonical flags; when inverting an IntFlag member (or boundary KEEP), also consider aliases.
(cherry picked from commit 59f009e)

Co-authored-by: Ethan Furman <ethan@stoneleaf.us>
@The-Compiler
Copy link
Contributor Author

Thanks @ethanfurman for the quick fix! I can confirm this fixes things for my project (qutebrowser) as well. I suppose this can be closed then?

@benburrill
Copy link

benburrill commented Jun 10, 2023

May be worth noting that there are still a few subtle differences between the new Flag behavior and the behavior from 3.10.

In Python 3.10, ~Flag.Mask == Flag(0), and Flag(252) would have raised a ValueError.
In Python 3.11.3, ~Flag.Mask == Flag(0) == Flag(252) due to the behavior of CONFORM
In Python 3.11.4, ~Flag.Mask == Flag(0) != Flag(252) (STRICT is still not strict enough to restore 3.10 behavior)
With @ethanfurman's fix, ~Flag.Mask == Flag(252) != Flag(0).

I think this change is probably fine for most cases, but it would be a good idea to add some more test cases.

@ethanfurman
Copy link
Member

Good catch, @benburrill -- I definitely want ~Flag.Mask == Flag(0).

ambv pushed a commit that referenced this issue Jul 5, 2023
…xist. (GH-105542) (#105572)

gh-105497: [Enum] Fix Flag inversion when alias/mask members exist. (GH-105542)

When inverting a Flag member (or boundary STRICT), only consider other canonical flags; when inverting an IntFlag member (or boundary KEEP), also consider aliases.
(cherry picked from commit 59f009e)

Co-authored-by: Ethan Furman <ethan@stoneleaf.us>
ambv pushed a commit that referenced this issue Jul 5, 2023
…xist. (GH-105542) (#105571)

When inverting a Flag member (or boundary STRICT), only consider other canonical flags; when inverting an IntFlag member (or boundary KEEP), also consider aliases.
(cherry picked from commit 59f009e)

Co-authored-by: Ethan Furman <ethan@stoneleaf.us>
@ambv
Copy link
Contributor

ambv commented Jul 5, 2023

I merged the 3.12 and 3.11 backports. To follow up on the ~Flag.Mask issue, I ran some tests to see what changed over time.

Expected results

~Flag.A=<Flag.B: 2>
~Flag.A.value=2
~Flag.A is Flag.B=True
~Flag.A == Flag.B=True

~Flag.Mask=<Flag.0: 0>
~Flag.Mask.value=0
~Flag.Mask is Flag(0)=True
~Flag.Mask == Flag(0)=True

3.10.12

As expected

3.11.0 - 3.11.3

As expected

3.11.4

~Flag.A raises ValueError: <flag 'Flag'>: no members with value 252

~Flag.Mask=<Flag: 0>
~Flag.Mask.value=0
~Flag.Mask is Flag(0)=True
~Flag.Mask == Flag(0)=True

3.11 after 3f244b2

~Flag.A=<Flag.B: 2>
~Flag.A.value=2
~Flag.A is Flag.B=True
~Flag.A == Flag.B=True

~Flag.Mask=<Flag: 252>
~Flag.Mask.value=252
~Flag.Mask is Flag(0)=False
~Flag.Mask == Flag(0)=False

3.12.0 alpha 1 - alpha 7

As expected

3.12.0 beta 1 - beta 3

~Flag.A raises ValueError: <flag 'Flag'>: no members with value 252

~Flag.Mask=<Flag: 0>
~Flag.Mask.value=0
~Flag.Mask is Flag(0)=True
~Flag.Mask == Flag(0)=True

3.12.0 beta after 74d84cf

~Flag.A=<Flag.B: 2>
~Flag.A.value=2
~Flag.A is Flag.B=True
~Flag.A == Flag.B=True

~Flag.Mask=<Flag: 252>
~Flag.Mask.value=252
~Flag.Mask is Flag(0)=False
~Flag.Mask == Flag(0)=False

~Flag.Mask needs an urgent fix

In sum, before the last beta of 3.12 we need to restore behavior of ~Flag.Mask such that the fix lands in 3.12.0 unimpeded. As such, I'm marking this as a release blocker for beta 4.

ambv pushed a commit that referenced this issue Jul 11, 2023
…106468)

For example:

    class Flag(enum.Flag):
        A = 0x01
        B = 0x02
        MASK = 0xff

    ~Flag.MASK is Flag(0)
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jul 11, 2023
…ist (pythonGH-106468)

For example:

    class Flag(enum.Flag):
        A = 0x01
        B = 0x02
        MASK = 0xff

    ~Flag.MASK is Flag(0)
(cherry picked from commit 95b7426)

Co-authored-by: Ethan Furman <ethan@stoneleaf.us>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jul 11, 2023
…ist (pythonGH-106468)

For example:

    class Flag(enum.Flag):
        A = 0x01
        B = 0x02
        MASK = 0xff

    ~Flag.MASK is Flag(0)
(cherry picked from commit 95b7426)

Co-authored-by: Ethan Furman <ethan@stoneleaf.us>
Yhg1s pushed a commit that referenced this issue Jul 11, 2023
…xist (GH-106468) (#106620)

gh-105497: [Enum] Fix flag mask inversion when unnamed flags exist (GH-106468)

For example:

    class Flag(enum.Flag):
        A = 0x01
        B = 0x02
        MASK = 0xff

    ~Flag.MASK is Flag(0)
(cherry picked from commit 95b7426)

Co-authored-by: Ethan Furman <ethan@stoneleaf.us>
Yhg1s pushed a commit that referenced this issue Jul 11, 2023
…xist (GH-106468) (#106621)

gh-105497: [Enum] Fix flag mask inversion when unnamed flags exist (GH-106468)

For example:

    class Flag(enum.Flag):
        A = 0x01
        B = 0x02
        MASK = 0xff

    ~Flag.MASK is Flag(0)
(cherry picked from commit 95b7426)

Co-authored-by: Ethan Furman <ethan@stoneleaf.us>
@AlexWaygood AlexWaygood added 3.11 only security fixes 3.12 bugs and security fixes labels Jul 21, 2023
@hugovk
Copy link
Member

hugovk commented Oct 2, 2023

Can this be release-blocker be closed now?

The linked PRs have been merged and backported.

Using Python 3.11.5, 3.12.0rc3 and 3.13.0a0 (heads/main:29c3a445d99, Oct 2 2023, 11:14:10), I get the expected Flag.B with:

import enum

class Flag(enum.Flag):
    A = 0x01
    B = 0x02
    Mask = 0xff

print(~Flag.A)

@benburrill
Copy link

Didn't realize there was a patch. At the risk of stating the obvious, I'll point out that Python 3.10 behavior is still not restored. For example:

class Example(enum.Flag):
    A = 0b001
    B = 0b110

In Python 3.10, ~Example.A == Example.B
In Python 3.11.3, ~Example.A == Example(0)
In Python 3.11.4, ~Example.A == Example.B
In Python 3.11.5, ~Example.A == Example(0)

Again, arguably either behavior is fine, but the Python 3.10 way is more intuitive, and there are use cases for weird flags that are not (at least fully) single-bit supported.

And also, as I mentioned earlier, in Python 3.10 Flag(252) from the previous example would have raised ValueError, and it still does not even in strict mode -- but I suppose that's kinda a different issue.

I can understand losing some of the Python 3.10 functionality in favor of performance, but I think we need to be more clear about what we want to keep and what the intended semantics of the new enums are.

@benburrill
Copy link

benburrill commented Oct 2, 2023

It occurs to me also that this new patch breaks DNE. Sure DNE was technically violated in 3.11.4 since ~Flag.A raised ValueError, and perhaps in other ways too, but in every version prior to 3.11.5 (I tested 3.10, 3.11.3, 3.11.4, and I assume also (didn't actually test) in Ethan's first patch where ~Flag.Mask == Flag(252)), it was the case that ~~Flag.Mask == Flag.Mask.

In Python 3.11.5 that is no longer true. Instead ~~Flag.Mask == Flag(3) != Flag.Mask

@ethanfurman
Copy link
Member

What is DNE?

@benburrill
Copy link

Double negation inversion elimination.

@ethanfurman
Copy link
Member

The 3.10 behavior is maintained with boundary=KEEP or boundary=EJECT.

class Example(enum.Flag, boundary=KEEP):
    A = 0b001
    B = 0b110
>>> Example.A
<Example.A: 1>

>>> ~Example.A
<Example.B: 6>

>>> ~~Example.A
<Example.A: 1>

@benburrill
Copy link

benburrill commented Oct 3, 2023

Good point, although the now default STRICT boundary is the closest to Python 3.10's behavior overall (albeit not identical). Also ~~Example.A is a bad example for the double-negation issue specifically, since that'd work even with STRICT. ~~Flag.Mask and ~~Example.B exhibit the behavior in question (but similarly to ~Example.A == Example.B, they do work as expected with KEEP and EJECT as you point out)

@ethanfurman
Copy link
Member

@benburrill: any other concerns with this patch, or are we good?

@benburrill
Copy link

benburrill commented Oct 4, 2023

My concern that the default STRICT boundary mode does not match the 3.10 behavior, and that this patch has resulted in further disparity has not really been addressed. The KEEP and EJECT boundary modes can't be used in previous versions and are both effectively just IntFlag. Thankfully IntFlag is backwards compatible, and does provide a way to write cross-version compatible code in cases where the version-dependent default behavior of Flag is problematic. However, I don't think this is an ideal situation.

To reiterate, I acknowledge that these issues (which really only show up if you have flags that are not fully single-bit-supported) are somewhat esoteric, and considering that enum has deliberately broken backwards compatibility in various other ways for Python 3.11, these few extra quirks are probably not going to break all that much now that the ValueError issue is resolved. Although I think the old behavior was more intuitive, personally I am fine with the new behavior so long as IntFlag remains as it is. However, I think the documentation is lacking on the differences between the FlagBoundary modes and what the intended behavior really is. The documentation says that non-canonical flags are excluded from iteration, but not much beyond that.

I also have some concerns about the Python release process. I'm not sure how, while this issue was marked as a release-blocker, it was possible for release candidates, let alone a full 3.X version to be released before the release-blocker status was removed. Sure, the ValueError issue has been fixed, but that was true even before the issue was marked as release-blocker.

If my concerns have drifted too far from the original focus of this issue (the ValueError problem), I'm fine with closing this issue and opening a new one. My biggest concern though, more than any specific issue, is on stabilizing enum on something consistent, understandable, explicitly documented, and preferably similar to how it was in 3.10. I don't want to see enum's behavior change every patch release, so I'm hesitant to pursue some of these smaller quirks at this point. I think the most immediate issue is one of documentation so that in the areas where enum differs across versions we can be more clear as to what is a bug and what is an intended feature / "advancement" of the new enums.

@ethanfurman
Copy link
Member

Closing as original concern has been addressed. Please open a new issue if any issues remain in 3.13.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes 3.12 bugs and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
Development

No branches or pull requests

6 participants