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

implementation of __or__ in enum.auto #73780

Closed
magu mannequin opened this issue Feb 17, 2017 · 10 comments
Closed

implementation of __or__ in enum.auto #73780

magu mannequin opened this issue Feb 17, 2017 · 10 comments
Assignees
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@magu
Copy link
Mannequin

magu mannequin commented Feb 17, 2017

BPO 29594
Nosy @ethanfurman, @serhiy-storchaka, @JulienPalard
Files
  • test.py: altered ~/cpython/Lib/enum.py
  • 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-03-02.00:06:45.221>
    created_at = <Date 2017-02-17.17:59:32.754>
    labels = ['3.7', 'type-feature', 'library']
    title = 'implementation of __or__ in enum.auto'
    updated_at = <Date 2017-03-02.00:06:45.220>
    user = 'https://bugs.python.org/magu'

    bugs.python.org fields:

    activity = <Date 2017-03-02.00:06:45.220>
    actor = 'ethan.furman'
    assignee = 'ethan.furman'
    closed = True
    closed_date = <Date 2017-03-02.00:06:45.221>
    closer = 'ethan.furman'
    components = ['Library (Lib)']
    creation = <Date 2017-02-17.17:59:32.754>
    creator = 'magu'
    dependencies = []
    files = ['46646']
    hgrepos = []
    issue_num = 29594
    keywords = []
    message_count = 10.0
    messages = ['288029', '288145', '288149', '288150', '288395', '288396', '288743', '288774', '288775', '288779']
    nosy_count = 4.0
    nosy_names = ['ethan.furman', 'serhiy.storchaka', 'mdk', 'magu']
    pr_nums = []
    priority = 'normal'
    resolution = 'rejected'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue29594'
    versions = ['Python 3.6', 'Python 3.7']

    @magu
    Copy link
    Mannequin Author

    magu mannequin commented Feb 17, 2017

    At the moment it is only possible to combine flags that already exist:

    from enum import *
    class Foo(Flag):
        A = auto()      # 1
        B = auto()      # 2
        AB = A | B      # 3 (1 | 2)
        AC = auto() | A # Fails, but should be 5 (1 | 4)
        ABD = auto() | A | B # Just taking it one step further to make a point, 11 (1 | 2 | 8)

    It would be nice to have this for cases when C only makes sense in combination with A but not on its own.

    A solution to achieve this one would need to change two things in ~/cpython/Lib/enum.py

    First extend class auto by:
    class auto:
    """
    Instances are replaced with an appropriate value in Enum class suites.
    """
    value = _auto_null
    or_value = 0

        def __or__(self, other):
    	""" Postpone the real or operation until value creation in _EnumDict """
    		 
            self.or_value |= other
            return self

    And second change one line in _EnumDict:
    value = value.value
    changes to:
    value = value.value | value.or_value

    Some simple tests show the expected results:
    print(repr(Foo.A)) # A - 1
    print(repr(Foo.B)) # B - 2
    print(repr(Foo.AB)) # AB - 3
    print(repr(Foo.AC)) # AC - 5
    print(repr(Foo.A | Foo.AC)) # AC - 5
    print(repr(Foo.A & Foo.AC)) # A - 1
    print(repr(Foo.ABD)) # ABD - 11

    Would it make sense to enhance python enums with that functionality?

    @magu magu mannequin added 3.7 (EOL) end of life stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Feb 17, 2017
    @magu magu mannequin changed the title implement __or__ in enum.auto implementation of __or__ in enum.auto Feb 18, 2017
    @JulienPalard
    Copy link
    Member

    Your implementation looks right, but I don't see the point of defining combinations AB, AC, ABD in the Foo enum. Foo may only define A, B, C, D and outside of Foo anyone can build any needed combinations.

    This way it looks clear in the Foo declaration (4 lines, 4 auto()). Did I missed a usefull usage of declaring combination inside the enum?

    @magu
    Copy link
    Mannequin Author

    magu mannequin commented Feb 19, 2017

    One made-up use-case would be:

    class LogLevel(Flags):
        start = auto()
        log1 = start | auto()
        log2 = start | auto()
    
    
    def fun(flags, *args):
        if start in flags:
            # open log file
    
        if log1 in flags:
           # Log important thing 1
    
        if log2 in flags:
           # Log important thing 2
        
        if start in flags:
           # close log file

    Alternatively the same could be achieved using the existing capabilities with:

    class LogLevel(Flags):
         start = auto()
         _log1 = auto()
         log1 = start | _log1
         _log2 = auto()
         log2 = start | _log2

    Which is less clear imho and could potentially a problem if somebody uses LogLevel._log2

    Another alternative would be that within the function we would check for all cases. eg:

    if (start in flags) or (log1 in flags) or (log2 in flags):

    Which leads to less clear code and makes the code less maintainable when log3 gets introduced. In the existing case we need to remember to change the if clause both when opening and closing the file. After the proposed change we only need to change the enum.

    I'm sure there are more use-cases for it. The one I'm using it for is a bit more convoluted that's why I'm not presenting it here.

    @serhiy-storchaka
    Copy link
    Member

    Just make C and D protected or private.

    class Foo(Flag):
        A = auto()
        B = auto()
        AB = A | B
        _C = auto()
        __D = auto()
        AC = A | _C
        ABD = A | B | __D

    @ethanfurman
    Copy link
    Member

    @Julian: Giving flag combinations their own name can be useful. For example, instead of seeing Color.GREEN|RED one can see Color.YELLOW .

    @marc: I'm not convinced this is a needed change as it doesn't seem to be a common method, nor one that cannot be easily implemented independently (perhaps as a recipe in the docs).

    Are you aware of other enumerations that take this approach?

    @ethanfurman
    Copy link
    Member

    I also think using leading underscores is a better way to signal that a particular value is "weird".

    @ethanfurman
    Copy link
    Member

    aenum 2.0 [1] has been released. Because it also covers Python 2.7 I had to enhance its auto() to cover |, &, ^, and ~ so that Enum classes could be properly created.

    At this moment your choices are to use odd naming or aenum (with its enhanced auto).

    [1] https://pypi.python.org/pypi/aenum

    @ethanfurman ethanfurman self-assigned this Mar 1, 2017
    @magu
    Copy link
    Mannequin Author

    magu mannequin commented Mar 1, 2017

    @Ethan, didn't know about aenum, thanks for showing it to me. However it doesn't seem to support the behavior I'm after (or I'm doing something wrong)

    import aenum

    try:
    class Foo(aenum.Flag):
    a = aenum.auto()
    b = a | aenum.auto()
    except Exception as err:
    print(err)

    try:
    class Bar(aenum.Flag):
    a = aenum.auto()
    b = aenum.auto() | a
    except Exception as err:
    print(err)

    results in
    unsupported operand type(s) for |: 'int' and 'auto'
    exceptions must derive from BaseException

    where the latter might be a bug in the implementation.

    I do realize that I'm stuck with this for the moment. My motivation with opening this thread was that I was wondering if such a feature would be worthwhile for the community. In case there is interest in this feature I would try to run the unit test and follow all the steps to try to push it through. However I save myself the work in case the community decides that the implementation is not worth it. Which would also be fine with me, as I monkey patched it for my code - so no problem on my end.

    @serhiy-storchaka
    Copy link
    Member

    I don't think it is worthwhile. Using underscored names looks pretty pythonic to me.

    @ethanfurman
    Copy link
    Member

    Serhiy, agreed. Closing.

    Marc, thanks, I see I missed supporting non-auto() combinations. Feel free to open an issue for that at:

    https://bitbucket.org/stoneleaf/aenum

    Either way I'll get that fixed.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants