This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: locals().update doesn't work in Enum body, even though direct assignment to locals() does
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ethan.furman Nosy List: bup, ethan.furman, josh.r
Priority: normal Keywords: patch

Created on 2018-09-20 12:39 by Antony.Lee, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 23725 merged ethan.furman, 2020-12-09 22:28
Messages (12)
msg325864 - (view) Author: Antony Lee (Antony.Lee) * Date: 2018-09-20 12:39
A quick check suggests that enum entries can be programmatically created by assigning to locals() in the Enum body:

   class E(Enum): locals()["a"] = 1
   E.a  # -> <E.a: 'a'>

However, using locals().update(...) doesn't, and silently does the wrong thing:

   class E(Enum): locals().update({"a": "a"})
   E.a  # -> 'a'

(Yes, in this simple case, I could just use the functional API (`E = Enum("E", [("a", "a")])`), but the above is simpler if I also want e.g. to define methods for the Enum.
msg325881 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2018-09-20 14:28
`locals()` returns the dictionary being used (an _EnumDict) and direct assignment uses the `__setitem__` method, which has been overridden -- and it is the only one; so `update()`, etc., still have normal dict meanings and not Enum ones.

Next step:  compile list of all methods that _EnumDict should override.

Or just say it's not supported.

Antony, can you give a more detailed use-case?  Meaning an actual example, please.
msg325894 - (view) Author: Antony Lee (Antony.Lee) * Date: 2018-09-20 16:07
I have a personal helper function for writing (Qt) GUIs that generates ComboBoxes from Enums; essentially something like

    class Choices(Enum):
        choice1 = "text for choice 1"
        choice2 = "text for choice 2"

    def callback(choice: Choices):
        # do stuff based on choice
        pass

    create_combobox(Choices, callback=callback)

I'm not including the actual code for create_combobox because it's not particularly relevant here.

So far, if I wanted to add methods to the Choice enum (e.g. for simplifying the code in `callback`), I could do it directly; if I wanted to dynamically generate the list of choices, I could also do it (by using the functional API).  But to do both, it would have been nice to use the locals().update approach in the bug report above.  Instead, I need to do something like

    class MethodsMixin(Enum):
        def method(self): ...

    ChoicesWithMethods = MethodsMixin("ChoicesWithMethods", [<the choices>])

(As a side note, I originally thought I could do the inheritance in the opposite direction

    Choices = Enum("Choices", [...])

    class ChoicesWithMethods(Choices):
        def method(self): ...

but that fails because Choices is a final class.  It would be nice if it was still possible to inherit from Choices *as long as only methods are added, rather than new members* (I understand why adding new members is problematic).  But this is really a side point.)

Making _EnumDict actually support update() and every other relevant method is actually not particularly difficult: I think you just need to make it inherit from (collections.abc.MutableMapping, dict) (in that order); this is exactly the approach used (since a while ago) by matplotlib's RcParams class (https://github.com/matplotlib/matplotlib/blob/v3.0.0/lib/matplotlib/__init__.py#L783).
msg325994 - (view) Author: Josh Rosenberg (josh.r) * (Python triager) Date: 2018-09-21 15:40
The documentation for locals ( https://docs.python.org/3/library/functions.html#locals ) specifically states:

Note: The contents of this dictionary should not be modified; changes may not affect the values of local and free variables used by the interpreter. 

The docstring for locals is similar, making it clear that any correlation between the returned dict and the state of locals if *either* is subsequently modified is implementation dependent, subject to change without back-compat concerns; even if we made this change, we've given ourselves the freedom to undo it at any time, which makes it useless to anyone who might try to rely on it.

The fact that even locals()["a"] = 1 happens to work is an implementation detail AFAICT; normally, locals() is and should remain read-only (or at least, modifications don't actually affect the local scope aside from the dict returned by locals()).

I'm worried that making _EnumDict inherit from collections.abc.MutableMapping in general would slow down Enums (at the very least creation, I'm not clear on whether _EnumDict remains, hidden behind the mappingproxy, for future lookups on the class), since MutableMapping would introduce a Python layer of overhead to most calls.

I'm also just not inclined to encourage the common assumption that locals() returns a dict where mutating it actually works, since it usually doesn't.
msg326001 - (view) Author: Antony Lee (Antony.Lee) * Date: 2018-09-21 16:27
> encourage the common assumption that locals() returns a dict where mutating it actually works, since it usually doesn't.

It does at global and class scope, not at function scope.

FWIW, PEP558 (admittedly not accepted yet) proposes to modify the documentation for the semantics of locals() at class-scope to match the actual behavior (https://www.python.org/dev/peps/pep-0558/#class-scope):

    At class scope [...] changes to the returned mapping must change the values bound to local variable names in the execution environment.

> I'm worried that making _EnumDict inherit from collections.abc.MutableMapping in general would slow down Enums (at the very least creation, I'm not clear on whether _EnumDict remains, hidden behind the mappingproxy, for future lookups on the class), since MutableMapping would introduce a Python layer of overhead to most calls.

"Normal" calls won't: __setitem__ / __getitem__ stays what they are, they were implemented in Python and stay implemented in Python.  Whoever calls update() will go through an extra Python layer, but there's not much you can do about that.

Alternatively, if you *really* don't want to support the MutableMapping API, at least it should be disabled (e.g. by adding stub implementations of update(), etc. that throw NotImplementedError).  Again performance-wise this would only affect those who try to call these methods.

As a side note, I would be curious to see any realistic code where the performance of enum creation turns out to be critical.  I don't think(?) the _EnumDict stays afterwards, at least PEP558 (which supposedly corresponds to the actual current behavior) also states "The mapping returned by locals() will not be used as the actual class namespace underlying the defined class (the class creation process will copy the contents to a fresh dictionary that is only accessible by going through the class machinery)."
msg326044 - (view) Author: Dan Snider (bup) * Date: 2018-09-21 21:26
It's working as intended. locals() and vars() simply returns the current frame's f_locals. In functions, modifying this usually accomplishes nothing useful because the code object has OPTIMIZED and NEWLOCALS flags set, meaning local variables are looked up or set via the LOAD_FAST and STORE_FAST opcodes (respectively) which doesn't even look in the f_locals mapping. In this case, vars() and locals() will build a new dict[*] and fill it with the frame's fastlocals and unpack any closure cells into it.

The code object used for class bodies however is special and actually *does* use the mapping in f_locals, which for for classes ultimately built by builtins.__build_class__ (aka classes built with a `class` statement) will be whatever the metaclass's __prepare__ returns, which in the case of enums is an enum._EnumDict instance. 

So that's why metaclasses are so powerful. You don't even need to use a dictionary subclass as the class namespace, since the STORE_NAME opcode will use PyObject_SetItem; however type.__new__ will make you cast it to a dict, and even the dict that is wrapped by a MappingProxy after the class has been created will be a copy anyway. 

So anyway, there's nothing actually wrong with the current behavior. dict.update never calls `self.__getitem__`, and since `_EnumDict.__setitem__` is where all of the magic happens regular dict.update won't trigger it. I agree though that adding an update method would be nice though and can be done in just a few lines of code.

    import enum
    import sys

    def local_update(it=(), **kws):
        self = sys._getframe(1).f_locals
        d = dict(it, **kws)
        for k, v in d.items():
            self[k] = v
            
    class MyEnum(enum.Enum):
        local_update(a=1, b=2)
        
    assert MyEnum.a.value == 1

[*] it doesn't actually build a new one every time but the only practical purpose with the NEWLOCALS code.co_code flag set is for introspection with vars(), locals(), and sys._getframe
msg326085 - (view) Author: Antony Lee (Antony.Lee) * Date: 2018-09-22 08:39
> I agree though that adding an update method would be nice though and can be done in just a few lines of code.

Again, this can be done just be inheriting the methods from MutableMapping.

In fact even now one can just write

    class E(Enum): MutableMapping.update(locals(), {"a": 1})

and this will do the "right" thing but that's hardly an obvious way to do it...
msg376814 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2020-09-13 00:02
Antony,

My apologies for the delay.

What I would like to see is a real example of how you would use this new feature if it were implemented.  I'm guessing it would look something like:

    class MyEnum(Enum):

        locals.update(*some magic here*)

        def a_method(self):
            ...

Am I right?  If yes, what does *some magic here* look like?  If no, what would your code actually look like?
msg376833 - (view) Author: Antony Lee (Antony.Lee) * Date: 2020-09-13 11:46
To be honest, I don't really remember what exact use case I had in my mind 2 years ago (as I probably worked around it in one way or another).  However, one example that I can think of (and that I have actually implemented before) is auto-conversion of C #defines from a C header file to a Python-level enum (e.g. for semi-automatic generation of a ctypes wrapper):

    # A general header parser (untested, just an example)
    def parse_defines(header_file):
        d = {}
        for line in header_file:
            if line.startswith("#define"):
                _, k, v = line.split()
                d[k] = int(v)
        return d

    # Now wrapping a specific C library
    foo_defines = parse_defines("foo.h")

    class Foo(Enum):
        locals().update({k: v for k, v in foo_defines.items() if k.startswith("FOO_")})

        def some_method(self):
            ...
            # e.g. call a C function that takes a FOO_* as parameter.

Obviously I could always just replace the method by a free function, but that's true for (nearly) all methods.  In other words, it seems a bit "unfair" that it is easy to define methods on enums where all options are explicitly listed, but very hard(?) to do so on enums with programatically defined options.
msg382809 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2020-12-09 22:31
Okay, you convinced me.  I would ask two things, though:

- use vars() instead of locals()
- split the one-liner  ;)

    class Foo(Enum):
        vars().update({
                k: v
                for k, v in foo_defines.items()
                if k.startswith('FOO_')
                })
        def some_method(self):
            # do something
msg382845 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2020-12-10 21:07
New changeset a65828717982e6a56382d7aff738478f5b5b25d0 by Ethan Furman in branch 'master':
bpo-34750: [Enum] add `_EnumDict.update()` support (GH-23725)
https://github.com/python/cpython/commit/a65828717982e6a56382d7aff738478f5b5b25d0
msg382847 - (view) Author: Antony Lee (Antony.Lee) * Date: 2020-12-10 21:27
Thanks!
History
Date User Action Args
2022-04-11 14:59:06adminsetgithub: 78931
2020-12-10 21:27:38Antony.Leesetnosy: - Antony.Lee
2020-12-10 21:27:30Antony.Leesetnosy: ethan.furman, Antony.Lee, josh.r, bup
messages: + msg382847
2020-12-10 21:07:59ethan.furmansetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2020-12-10 21:07:13ethan.furmansetmessages: + msg382845
2020-12-09 22:31:04ethan.furmansetmessages: + msg382809
2020-12-09 22:28:13ethan.furmansetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request22586
2020-12-09 22:22:57ethan.furmansetstage: needs patch
type: enhancement
versions: + Python 3.10, - Python 3.8
2020-09-13 11:46:44Antony.Leesetmessages: + msg376833
2020-09-13 00:02:05ethan.furmansetmessages: + msg376814
2018-09-22 08:39:30Antony.Leesetmessages: + msg326085
2018-09-21 21:26:46bupsetnosy: + bup
messages: + msg326044
2018-09-21 16:27:15Antony.Leesetmessages: + msg326001
2018-09-21 15:40:56josh.rsetnosy: + josh.r
messages: + msg325994
2018-09-20 16:07:17Antony.Leesetmessages: + msg325894
2018-09-20 14:28:05ethan.furmansetversions: + Python 3.8, - Python 3.7
nosy: + ethan.furman

messages: + msg325881

assignee: ethan.furman
2018-09-20 12:39:23Antony.Leecreate