python / cpython

The Python programming language
https://www.python.org
Other
61.97k stars 29.8k forks source link

Provide enum.unique class decorator #62242

Closed ncoghlan closed 11 years ago

ncoghlan commented 11 years ago
BPO 18042
Nosy @warsaw, @ncoghlan, @vstinner, @ezio-melotti, @ethanfurman, @phmc
Dependencies
  • bpo-17947: Code, test, and doc review for PEP-0435 Enum
  • Files
  • unique.stoneleaf.01.patch
  • unique.stoneleaf.02.patch
  • 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: ```python assignee = 'https://github.com/ethanfurman' closed_at = created_at = labels = ['type-feature', 'library'] title = 'Provide enum.unique class decorator' updated_at = user = 'https://github.com/ncoghlan' ``` bugs.python.org fields: ```python activity = actor = 'python-dev' assignee = 'ethan.furman' closed = True closed_date = closer = 'python-dev' components = ['Library (Lib)'] creation = creator = 'ncoghlan' dependencies = ['17947'] files = ['30722', '30730'] hgrepos = [] issue_num = 18042 keywords = ['patch'] message_count = 14.0 messages = ['189854', '189881', '189883', '189884', '189890', '189892', '189894', '190807', '191450', '191982', '191992', '192041', '192165', '193340'] nosy_count = 8.0 nosy_names = ['barry', 'ncoghlan', 'vstinner', 'ezio.melotti', 'eli.bendersky', 'ethan.furman', 'python-dev', 'pconnell'] pr_nums = [] priority = 'normal' resolution = 'fixed' stage = 'resolved' status = 'closed' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue18042' versions = ['Python 3.4'] ```

    ncoghlan commented 11 years ago

    Another attempt at tackling the "but I want to ensure my enum values are unique" problem that PEP-435 deliberately chose not to handle. My previous suggestion (in bpo-17959) was rightly rejected due to the other problems it caused, but this idea is much cleaner and simpler.

    All we would need to do is provide the following class decorator in the enum module:

        def unique(new_enum):
            for name, member in new_enum.__members__.items():
                if name != member.name:
                    msg = "Alias {!r} for {!r} not permitted in unique Enum"
                    raise TypeError(msg.format(name, member))
            return new_enum

    Used as so:

    >>> @enum.unique
    ... class MyEnum(enum.Enum):
    ...     a = 1
    ...     b = 2
    ...     c = 1
    ... 
    Traceback (most recent call last):
      File "<stdin>", line 2, in <module>
      File "<stdin>", line 6, in unique
    TypeError: Alias 'c' for <MyEnum.a: 1> not permitted in unique Enum
    ethanfurman commented 11 years ago

    This is certainly an effective method, but it places safety off by default. I would rather have a system that was from duplicates by default but had an easy override.

    The method I had in place in my original code was something like:

    class Color(Enum, options=DUPLICATES):
        red = 1
        green = 2
        blue = 3
        grene = 2

    Without the DUPLICATES option, the above class would raise an error. Safe(r) by default, easy override.

    If my suggestion doesn't fly, we should definitely put Nick's in.

    ncoghlan commented 11 years ago

    I take Guido's acceptance of the PEP (and the discussion in the previous issue) as meaning the default behaviour (allowing aliases) is no longer up for debate. Hence this suggestion to offer a self-documenting way to opt in to the more restrictive variant.

    ethanfurman commented 11 years ago

    I'm not giving up hope yet. Plenty of Python features no longer work the way they did when their PEP was accepted. ;)

    ncoghlan commented 11 years ago

    You don't generally see reversals of decisions where Guido has made an explicit choice based on consistency with the rest of the language. The fact that aliases are permitted in enumerations by default is consistent with the normal behaviour of namespaces and dictionaries in general, so providing a way to opt in to the stricter checks is a better solution.

    The idea of passing flags or other configuration options to the metaclass is also rather ugly. Offering permissive behaviour by default with an easy way to opt in to additional restrictions is far more in keeping with the general "consenting adults" ethos of the language.

    ethanfurman commented 11 years ago

    Oh. Well, I like your decorator. :)

    ncoghlan commented 11 years ago

    Don't worry, compared to some of the ideas I've had (and rightfully had shot down) over the years, that one was positively sensible :)

    warsaw commented 11 years ago

    +1 for the decorator!

    ethanfurman commented 11 years ago

    I haven't seen any discouraging words regarding the decorator. If no one has any compelling reasons why it shouldn't be added, I'll craft a version and put it in (only real difference with Nick's would be catching all the duplicates at once instead of one at a time).

    ethanfurman commented 11 years ago

    unique() added to enum.py; tests added; docs updated.

    If no complaints within a few days I'll push them through.

    786d3f11-b763-4414-a03f-abc264e0b72d commented 11 years ago

    Sent some review comments. I'll be on a short vacation this weekend, so please wait at least until next week so I can review the changes. Also, Nick should definitely review this too :)

    ethanfurman commented 11 years ago

    Integrated comments.

    vstinner commented 11 years ago

    The documentation still contains an "Interesting example": UniqueEnum. I would prefer to only have one obvious way to get unique enum, so please just drop this example. Or at least, mention the new decorator in the example.

    1762cc99-3127-4a62-9baf-30c3d0f51ef7 commented 11 years ago

    New changeset 2079a517193b by Ethan Furman in branch 'default': closes bpo-18042 -- a unique decorator is added to enum.py http://hg.python.org/cpython/rev/2079a517193b