-
-
Notifications
You must be signed in to change notification settings - Fork 31.7k
An unexpected difference between dict and OrderedDict #71763
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
Comments
Consider the following code: $ cat x.py
from collections import OrderedDict
class X:
def items(self):
print('items')
return []
def keys(self):
print('keys')
return [] print(dict(X()))
print(OrderedDict(X())) When I run it under python 3.4, I get $ python3.4 x.py
keys
{}
keys
OrderedDict() but under python 3.5, I get $ python3.5 x.py
keys
{}
items
OrderedDict() Under 3.4 both dict and OrderedDict constructors call the keys() method of the underlying object (and then call __getitem__ repeatedly if keys() returns a non-empty list), but in 3.5 OrderedDict behavior changed and it calls the values() method instead. |
This does need to be fixed. FWIW, here is the relevant docstring from MutableMapping: |
Submit a patch to fix this. Hope it helps. |
I think this case (fast path for exact dict) is not needed at all. Exact dict is not ordered, and OrderedDict created from exact dict has nondetermined order (unless a dict has size 0 or 1). |
I didn't think about order. I just thought using concrete API may be faster. But after your comment, I tested the performance and it seems PyDict_Items makes it much slower (it does more work, eg, make tuples). So I agree it is not needed at all. |
I write a new version restoring the fast path for dict. It now uses PyDict_Next and seems to be much faster than the path using keys. [cpython]$ ./python -m timeit -s 'from collections import OrderedDict; d = {"a":1,"c":2,"b":3,"d":4}' 'OrderedDict(d)' |
After totally studying OrderedDict, I get a better understanding of what Serhiy means. So although we can get a better performance for dict, it's not needed at all. Remove the v3 patch. |
New changeset 70758c12e888 by Eric Snow in branch 'default': |
Thanks for pointing this out, Alexander. |
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: