bpo-31265: Compact OrderedDict (wip)#3193
Conversation
There was a problem hiding this comment.
ma_offset has the type 'n'.
There was a problem hiding this comment.
Open a separate issue for this feature.
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.
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.
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?
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 |
methane
force-pushed
the
compact-odict
branch
from
92e7811 to
7fde816
Compare
dict.fromkeys() doesn't support keyword argument again.
methane
force-pushed
the
compact-odict
branch
from
7fde816 to
ae43850
Compare
6 participants
https://bugs.python.org/issue31265