Skip to content

locals().update doesn't work in Enum body, even though direct assignment to locals() does #78931

Closed

Description

@anntzer
mannequin
BPO 34750
Nosy @ethanfurman, @MojoVampire, @mr-nfamous
PRs
  • bpo-34750: [Enum] add _EnumDict.update() support #23725
  • 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 2020-12-10.21:07:59.178>
    created_at = <Date 2018-09-20.12:39:23.864>
    labels = ['type-feature', 'library', '3.10']
    title = "locals().update doesn't work in Enum body, even though direct assignment to locals() does"
    updated_at = <Date 2020-12-10.21:27:37.999>
    user = 'https://github.com/anntzer'

    bugs.python.org fields:

    activity = <Date 2020-12-10.21:27:37.999>
    actor = 'Antony.Lee'
    assignee = 'ethan.furman'
    closed = True
    closed_date = <Date 2020-12-10.21:07:59.178>
    closer = 'ethan.furman'
    components = ['Library (Lib)']
    creation = <Date 2018-09-20.12:39:23.864>
    creator = 'Antony.Lee'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 34750
    keywords = ['patch']
    message_count = 12.0
    messages = ['325864', '325881', '325894', '325994', '326001', '326044', '326085', '376814', '376833', '382809', '382845', '382847']
    nosy_count = 3.0
    nosy_names = ['ethan.furman', 'josh.r', 'bup']
    pr_nums = ['23725']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue34750'
    versions = ['Python 3.10']

    Activity

    1. anntzer commented on Sep 20, 2018

      anntzermannequin
      MannequinAuthor

      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.

    2. added
      stdlibStandard Library Python modules in the Lib/ directory
      on Sep 20, 2018
    3. ethanfurman commented on Sep 20, 2018

      @ethanfurman
      Member

      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.

    4. self-assigned this
      on Sep 20, 2018
    5. anntzer commented on Sep 20, 2018

      anntzermannequin
      MannequinAuthor

      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).

    6. MojoVampire commented on Sep 21, 2018

      MojoVampiremannequin
      Mannequin

      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.

    7. anntzer commented on Sep 21, 2018

      anntzermannequin
      MannequinAuthor

      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, PEP-558 (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 PEP-558 (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)."

    8. mr-nfamous commented on Sep 21, 2018

      mr-nfamousmannequin
      Mannequin

      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

    9. anntzer commented on Sep 22, 2018

      anntzermannequin
      MannequinAuthor

      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...

    10. ethanfurman commented on Sep 13, 2020

      @ethanfurman
      Member

      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?

    11. anntzer commented on Sep 13, 2020

      anntzermannequin
      MannequinAuthor

      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.

    12. added
      type-featureA feature request or enhancement
      and removed on Dec 9, 2020
    13. ethanfurman commented on Dec 9, 2020

      @ethanfurman
      Member

      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
    14. ethanfurman commented on Dec 10, 2020

      @ethanfurman
      Member

      New changeset a658287 by Ethan Furman in branch 'master':
      bpo-34750: [Enum] add _EnumDict.update() support (GH-23725)
      a658287

    15. anntzer commented on Dec 10, 2020

      anntzermannequin
      MannequinAuthor

      Thanks!

    16. transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Labels

    3.10 (EOL)end of lifestdlibStandard Library Python modules in the Lib/ directorytype-featureA feature request or enhancement

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions