bpo-31265: Compact OrderedDict (wip) #3193
Conversation
Lib/test/test_sys.py
Outdated
There was a problem hiding this comment.
ma_offset has the type 'n'.
Objects/dictobject.c
Outdated
Objects/dictobject.c
Outdated
There was a problem hiding this comment.
Open a separate issue for this feature.
Objects/dictobject.c
Outdated
There was a problem hiding this comment.
Open a separate issue for this feature.
Adding support of keyword arguments slows down arguments parsing.
There was a problem hiding this comment.
OK, I'll stop sharing methods and override it only for keyword argument support of odict.
Objects/dictobject.c
Outdated
There was a problem hiding this comment.
Add separate type for reversed iterators.
There was a problem hiding this comment.
OK, and I'll move ma_offset to odictobject too.
Objects/dictobject.c
Outdated
There was a problem hiding this comment.
The implementation of dict already is complex. I think it would be better to keep the implementation of OrderedDict in separate file. In additional to long-term effects this makes making review much harder.
There was a problem hiding this comment.
But new OrderedDict uses some internal static functions and variables.
I don't want to make it extern functions.
Do you prefer #include "odictobject.h"?
There was a problem hiding this comment.
I created split version of this pullrequest: methane#9
As you can see, many private functions has "extern" linkage to be used from odictobject.c.
https://github.com/methane/cpython/pull/9/files#diff-b06f0b32d4365b3d104057b8d40c90f8R96
@serhiy-storchaka @rhettinger Which do you prefer?
Lib/test/test_ordered_dict.py
Outdated
There was a problem hiding this comment.
These tests test former bugs in C implementation. They should be kept for C implementation for ensuring that these bugs are not reappeared.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
92e7811 to
7fde816
Compare
dict.fromkeys() doesn't support keyword argument again.
7fde816 to
ae43850
Compare
https://bugs.python.org/issue31265