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.nonmember type-decays Flag values #120361

Closed
thejcannon opened this issue Jun 11, 2024 · 11 comments
Closed

enum.nonmember type-decays Flag values #120361

thejcannon opened this issue Jun 11, 2024 · 11 comments
Labels
type-bug An unexpected behavior, bug, or error

Comments

@thejcannon
Copy link
Contributor

thejcannon commented Jun 11, 2024

Bug report

Bug description:

>>> from enum import Flag, auto, nonmember
>>> 
>>> class DepType(Flag):
...     LIBRARY = auto()
...     SCRIPT = auto()
...     TEST = auto()
...     ALL = nonmember(LIBRARY | SCRIPT | TEST)
...  
>>> DepType.ALL
7
>>> type(DepType.ALL)
<class 'int'>
>>> DepType.LIBRARY | DepType.SCRIPT | DepType.TEST
<DepType.LIBRARY|SCRIPT|TEST: 7>
>>> type(DepType.LIBRARY | DepType.SCRIPT | DepType.TEST)
<flag 'DepType'>

this manifests by seeing a funky error on:

>>> DepType.LIBRARY in DepType.ALL
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: argument of type 'int' is not iterable

If this isn't a bug, it should at least be mentioned on the docs.

CPython versions tested on:

3.11

Operating systems tested on:

Linux

Linked PRs

@thejcannon thejcannon added the type-bug An unexpected behavior, bug, or error label Jun 11, 2024
@thejcannon
Copy link
Contributor Author

I don't see this functionality tested one way or another in cpython/Lib/test/test_enum.py so odds of it being undesired behavior have increased

@sobolevn
Copy link
Member

This does not look like a bug to me. It does what is advertised: ALL is not an enum member. But, why is it an int?

Because TEST / LIBRARY / etc are converted to enum values after class body is evaluated. Inside the enum's body they are still just ints. Proof:

>>> class DepType(Flag):
...     LIBRARY = auto()
...     TEST = auto()
...     print(LIBRARY, TEST, LIBRARY | TEST)
...     
1 2 3

So, ALL will just record their value and store it as a property.

I recommend doing something like:

>>> DepType.LIBRARY in DepType
True

This won't require to define ALL for this case.

But, I agree that adding a test case for this corner case might be a good idea.

@thejcannon
Copy link
Contributor Author

Because TEST / LIBRARY / etc are converted to enum values after class body is evaluated

Oh duh!

This won't require to define ALL for this case.

The real-world scenario is a bit more hoop-y:

def foo(dep_type: DepType = DepType.ALL):
    if DepType.LIBRARY in dep_type:
        do_thing()

So having the nice .ALL shorthand is great.

@ethanfurman
Copy link
Member

Why are you having .ALL not be a member? (Technically, it would be an alias.)

@sobolevn
Copy link
Member

But, the code is not correct :)

def foo(dep_type: DepType = DepType.ALL):
    if DepType.LIBRARY in dep_type:
        do_thing()

We now know that DepType.ALL is int. But, it is annotated as DepType. It should be:

def foo(dep_type: DepType | int = DepType.ALL):
    if not isinstance(dep_typ, DepType):
        dep_type = DepType(dep_typ) 
    if DepType.LIBRARY in dep_type:
        do_thing()

And it will do what you want :)

@thejcannon
Copy link
Contributor Author

Well in this case I want to be a nice easy DepType (I understand there's nuance here, but the point was that I don't want it to be a int (hence the bug report).

I think this might capture the intent best (albeit a little verbose):

class DepType(Flag):
    LIBRARY = auto()
    SCRIPT = auto()
    TEST = auto()

    ALL: "DepType"

DepType.ALL = DepType.LIBRARY | DepType.SCRIPT | DepType.TEST

@sobolevn
Copy link
Member

Demo for @ethanfurman's suggestion:

>>> class Example(Flag):
...             A = 1
...             B = 2
...             ALL = A | B
...             
>>> Example.ALL
<Example.ALL: 3>
>>> Example.A & Example.ALL
<Example.A: 1>
>>> Example.A & Example.B
<Example: 0>
>>> bool(Example.A & Example.B)
False

@ethanfurman
Copy link
Member

@thejcannon You're last example is exactly the same as if you had not used nonmember in the first place.

@thejcannon
Copy link
Contributor Author

thejcannon commented Jun 11, 2024

@thejcannon You're last example is exactly the same as if you had not used nonmember in the first place.

Not quite (see below) although:

Why are you having .ALL not be a member? (Technically, it would be an alias.)

Welp, I incorrectly assumed it'd show up in list(DepType) as it is a member. But I just tried it and I'm wrong.

Apparently my intuiting surrounding Flag enums is just ALL out of wack 😥 .

from enum import Flag, auto, nonmember


class DepType(Flag):
    LIBRARY = auto()
    SCRIPT = auto()
    TEST = auto()

    IMMEDIATE = nonmember(LIBRARY | SCRIPT | TEST)
    DELAYED: "DepType"
    ALIAS = LIBRARY | SCRIPT | TEST


DepType.DELAYED = DepType.LIBRARY | DepType.SCRIPT | DepType.TEST

print(type(DepType.IMMEDIATE))  # <class 'int'>
print(type(DepType.DELAYED))  # <flag 'DepType'>
print(type(DepType.ALIAS))  # <flag 'DepType'>
print(list(DepType))  # [<DepType.LIBRARY: 1>, <DepType.SCRIPT: 2>, <DepType.TEST: 4>]

So, looks like I do just want ALL = LIBRARY | SCRIPT | TEST and everything "Just Works ™️"

I'll leave bug report open for the added test cases, and if anyone wants to noodle if the docs could be more illuminating.

Otherwise, pack it up nothing to see here. Just bad intuition.

@ethanfurman
Copy link
Member

@thejcannon When Flag first came out, aliases were included in iterations -- that changed in 3.10 or 3.11.

ethanfurman pushed a commit that referenced this issue Jun 14, 2024
…GH-120364)

* gh-120361: Add `nonmember` test with enum flags inside to `test_enum`
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 14, 2024
…_enum` (pythonGH-120364)

* pythongh-120361: Add `nonmember` test with enum flags inside to `test_enum`
(cherry picked from commit 7fadfd8)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 14, 2024
…_enum` (pythonGH-120364)

* pythongh-120361: Add `nonmember` test with enum flags inside to `test_enum`
(cherry picked from commit 7fadfd8)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
sobolevn added a commit that referenced this issue Jun 14, 2024
…t_enum` (GH-120364) (#120512)

gh-120361: Add `nonmember` test with enum flags inside to `test_enum` (GH-120364)

* gh-120361: Add `nonmember` test with enum flags inside to `test_enum`
(cherry picked from commit 7fadfd8)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
sobolevn added a commit that referenced this issue Jun 14, 2024
…t_enum` (GH-120364) (#120511)

gh-120361: Add `nonmember` test with enum flags inside to `test_enum` (GH-120364)

* gh-120361: Add `nonmember` test with enum flags inside to `test_enum`
(cherry picked from commit 7fadfd8)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
@sobolevn
Copy link
Member

Thanks everyone!

mrahtz pushed a commit to mrahtz/cpython that referenced this issue Jun 30, 2024
…_enum` (pythonGH-120364)

* pythongh-120361: Add `nonmember` test with enum flags inside to `test_enum`
noahbkim pushed a commit to hudson-trading/cpython that referenced this issue Jul 11, 2024
…_enum` (pythonGH-120364)

* pythongh-120361: Add `nonmember` test with enum flags inside to `test_enum`
estyxx pushed a commit to estyxx/cpython that referenced this issue Jul 17, 2024
…_enum` (pythonGH-120364)

* pythongh-120361: Add `nonmember` test with enum flags inside to `test_enum`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants