GeezerVison

The meaning of "members" should be clarified. Specifically, the iteration intentionally omits aliases and compound members.

The tests should verify this.

I would describe it as "iterating over the bits of a flag value", because that's what it does. Using "bits" should be OK since Flag is documented as supporting bitwise operators.

I agree.

The _decompose function is heavyweight (fairly complex and includes sorting). I wouldn't expect this kind of overhead when iterating flag bits.

I can think of two efficient ways:

1. compare to all members of the Flag class, excluding 0 and non-power-of-2:

return (v for v in self.__class__ if v.value != 0 and (v.value & (v.value-1) == 0) and v in self)

2. walk through the bits with a helper

@staticmethod
def _bits(n):
    while n:
        b = n & (~n+1)
        yield b
        n ^= b

def __iter__(self):
    return (self.__class__(v) for v in self._bits(self._value_))

in fact _decompose is used in other places, like __invert__, making these operations really slow. Really the set of available bits should be available in a class-only attribute, like _all_.

proposal (2) is 2x faster than the implementation in this PR, for an enum with only about 6 values...

>>> from enum import Flag, auto
>>> class Foo(Flag):
...     NONE = 0
...     A = auto()
...     B = auto()
...     C = auto()
...     C2 = C
...     AB = A | B
...     D = auto()
...     E = auto()
...     F = auto()
...
>>> x = Foo.A | Foo.B | Foo.D | Foo.F
>>> import timeit
>>> timeit.timeit('list(x)', globals=locals(), number=10000)
0.6332780510000013

proposal (2):

>>> timeit.timeit('list(x)', globals=locals(), number=10000)
0.36078684099999236

I have become increasingly unhappy with the the multiple calls to _decompose. Your ideas (and others, too) are welcome.

I have become increasingly unhappy with the the multiple calls to _decompose. Your ideas (and others, too) are welcome.

I do have ideas on it, and intended to open a separate issue for refactoring _decompose() and make a PR replacing it with something simpler, faster, and less buggy. (E.g. use the _bits() iterator; and I noticed that all callers of it either use the members or extra_flags output, never both; and the computation of extra_flags would be trivial if we had an __all__ attribute on the class.)

Regarding bugs, when it's used by repr() or str(), I don't think it's intended that a lone compound value is represented as-is, while a compound plus other value is represented as a redundant "compound plus underlying bits":

>>> class Foo(Flag):
...     A = auto()
...     B = auto()
...     C = auto()
...     BC = B | C
>>> repr(Foo.BC)
'<Foo.BC: 6>'
>>> repr(Foo.A | Foo.BC)
'<Foo.BC|C|B|A: 7>'

I think the use of __iter__() is problematic given that __contains__() has different semantics.

__contains__() returns true for members with value 0 or compound members, and __iter__() does not. It's bad form for these not to match.

>>> class Foo(Flag):
...     NONE = 0
...     A = 1
...     B = 2
...     C = 4
...     C2 = C
...     AB = A | B
>>> x = Foo.A | Foo.B
>>> list(x)
[<Foo.B: 2>, <Foo.A: 1>]
>>> Foo.NONE in x
True
>>> Foo.AB in x
True

I'd argue that __contains__() is completely broken and no one wants that behavior. But assuming that can't be changed, using __iter__() to iterate the bits is probably not a good idea.

I agree that the zero-value flag should not be contained in a non-zero flag, but I have no problem with a compound flag showing up if its bits are present. You are welcome to try and convince me otherwise, but it will take more than "no one wants that behavior".

Hmm, I just learned that Flag.__contains__() is technically a subset operator:

>>> class Foo(Flag):
...     NONE = 0
...     A = 1
...     B = 2
...     C = 4
...     AB = A | B
>>> Foo.AB in (Foo.AB | Foo.C)
True
>>> (Foo.A|Foo.C) in (Foo.AB | Foo.C)
True

If that's the case, then actually it's OK for Foo.None in Foo.A to be True, since Foo.None is the empty set:

>>> set().issubset({1, 2})
True

I would not have made the API this way. __contains__ should be strictly for testing whether a bit is in the value (a Flag value is a set of enabled bits, in my mind), while issubset() and operator <= should be used for subset.