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
Creating dict from OrderedDict doesn't preserve order #78501
Comments
>>> from collections import OrderedDict
>>> od = OrderedDict([('a', 1), ('b', 2)])
>>> od.move_to_end('a')
>>> od
OrderedDict([('b', 2), ('a', 1)])
>>> dict(od)
{'a': 1, 'b': 2} This affects also PEP-468. >>> def f(**kwargs): return kwargs
...
>>> f(**od)
{'a': 1, 'b': 2} And PEP-520. >>> type('A', (), od).__dict__
mappingproxy({'a': 1, 'b': 2, '__module__': '__main__', '__dict__': <attribute '__dict__' of 'A' objects>, '__weakref__': <attribute '__weakref__' of 'A' objects>, '__doc__': None}) |
FWIW, the PEP-520 example isn't exactly right. PEP-520 is about the class definition namespace, not the namespace object passed to type(). That always does the right thing, since the default class definition namespace is dict (which happens to be ordered in 3.6+). That said, a test that explicitly calls type() with an OrderedDict, as you have shown, is still a good idea since type() always changes the namespace to a dict. Perhaps another good test of the same thing would be with a metaclass that returns an OrderedDict from __prepare__(): class Meta(type):
@classmethod
def __prepare__(meta, name, bases, **kwargs):
return OrderedDict()
class Spam(metaclass=Meta):
a = 1
b = 2
locals().move_to_end('a') Spam.__dict__ Just keep in mind that neither of those is specific to PEP-520. |
It is not relating to this issue: there are no conversion happened. |
There is a conversion. See builtin___build_class__ in Python/bltinmodule.c (and the LOAD_BUILD_CLASS target in Python/ceval.c). Specifically, the metaclass (e.g. the builtin type) is instantiated using the namespace from the class definition. The metaclass copies that namespace into a new dict. So the following two bits of code are equivalent: # using a class definition ns = OrderedDict()
class Meta(type):
def __prepare__(*args, **kwargs):
return ns
class Spam(metaclass=Meta):
a = 1
b = 2
ns.move_to_end('a') # using the metaclass directly ns = OrderedDict()
ns['a'] = 1
ns['b'] = 2
ns.move_to_end('a')
Spam = Meta('Spam', (object,), ns) In both cases Spam.__dict__ will be a proxy for a new dict copied from ns. |
Conversion is happens when type() (or metaclass()) is called. Why regression test for this bugfix need to use __prepare__? |
Right. So an extra test to cover the __prepare__ case is somewhat redundant. I only suggested it because type() is called implicitly and the test would make the conversion case clear. However, it was only a suggestion for you to consider, not something I considered necessary. :) I'm fine with not adding a test that uses __prepare__. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: