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
Make class definition namespace ordered by default #68442
Comments
Here's a work-in-progress patch that uses OrderedDict as the default class definition namespace (instead of dict). While the actual class namespace is a dict, the definition order is preserved on a new attribute on class objects: __definition_order__. This is useful for cases such as class decorators that would make use of the definition order to post-process the class. This is something that Guido okay'ed a while back. It may be a stretch to get this into 3.5, but the final change should be pretty small. It also depends on the C implementation of OrderedDict (see issue bpo-16991) which has not landed and may not make it in time. Note that the patch is still a bit buggy, partly due to bugs in the C OrderedDict patch. Also, the attached patch is based off the C OrderedDict patch, so I expect reitveld won't be able to handle it. The code on which the patch is based may be found in the "class-namespace" branch of https://pypi.python.org/pypi/cyordereddict. |
Here's a patch that drops adding definition_order. You can get the same effect by adding Ultimately, I'd prefer to keep |
types.prepare_class() also needs to be updated to use OrderedDict() by default. |
Per discussion on python-dev, I'm tabling the __definition_order__ part to 3.6. I'll open a thread on python-ideas on it later and open a new issue here if I get a positive response. So this issue is just about making OrderedDict the default namespace type for class definitions. |
Changing the default type of class dictionaries is a huge change. IMO it should be deferred to Python 3.6. |
This is not about changing the default type for class dictionaries. It is only for changing the default type used during class definition. Essentially, we are just changing the type of what is returned from |
This is *not* about changing the default type of class dictionaries (which |
Thanks for pointing out types.prepare_class. I've updated it. |
I've moved this to 3.6. Small as the patch might be, there just isn't enough urgency to warrant making use of an exception to get it into 3.5. If __definition_order__ were still on the table then I'd probably still push for 3.5. :) |
Here's a refresh of the patch that only sets the default definition namespace (and does not introduce __definition_order__). |
Patch review sent. The motivation for the change is relatively weak without __definition_order__, though :) |
Thanks. Yeah, I wanted to keep the patches separate for the sake of code review. I'll fold the changes into a single commit once everything's ready. |
Here's the full patch, including the addition of __definition_order__, tests, and docs. |
The patch looks good to me as a reference implementation, but I think it's worth writing up as a short PEP so there's a clear reference from the What's New documentation, and so developers of other implementations have a chance to review and comment on the change. |
Here's an updated patch that matches the accepted PEP. Most notably I've added a tp_deforder slot. This was necessary because subclasses should not inherit their parent's __definition_order__ and the ability to delete cls.__definition_order__ makes this problematic. I didn't see a way to do that without adding a new type slot. |
FTR: PEP-520 "Preserving Class Attribute Definition Order" (https://www.python.org/dev/peps/pep-0520/) |
post-review updates |
Any update on this? b1 is exactly 2 weeks away at this point. |
Brett Cannon added the comment:
Why do we need changes specific to classes, if dictionaries could be ordered by default? Issue bpo-27350: "Compact and ordered dict" by INADA Naoki |
Because we're not making ordered-by-default dicts a language specification level requirement, so portable code can't rely on them. However, the PEP at https://www.python.org/dev/peps/pep-0520/ is deliberately worded so that there's no requirement for the class body execution namespace to specifically be collections.OrderedDict - it just needs to be order preserving so that __definition_order__ can be populated. If someone reviews and merges the compact ordered dict patch, then this patch can likely be made a lot simpler. |
New changeset 635fd3912d4d by Eric Snow in branch 'default': |
New changeset 9aa5424fd1df by Eric Snow in branch 'default': |
Is this going to be added back? Should I add __definition_order__ to Enum? |
Nope, PEP-520 has been updated to drop __definition_order__ and instead state that cls.__dict__ is an ordered mapping. |
Not having a __definition_order__ could be a disadvantage for classes making use of __getattr__ for virtual attributes, such as Enum and proxy classes. With __definition_order__, and __dir__, Enum could present a coherent view; without it it's a big jumbled mess with many attributes the user never wrote. |
You should reopen the discussion on python-dev, since the PEP-520 has "Note: Since compact dict has landed in 3.6, __definition_order__ has The PEP status is now Final. |
The danger of sprints, and decisions being made without adequate input from affected parties. |
Ethan started a python-dev thread here: https://mail.python.org/pipermail/python-dev/2016-September/146358.html |
Reopening until the __definition_order__ question has been resolved. |
I don't think this is necessarily a blocker for beta 1 (since it can be treated as a bug fix for beta 2 if it's decided to restore the originally proposed behaviour), but it should definitely be resolved before beta 2. |
Do realize that the PEP author was there and made the decision along with Guido to not move forward with a new feature that has not seen the light of day in a stable release, so I don't think blaming the sprinting environment is entirely fair in this case. |
While I hadn't read the related thread at the point where I made that comment (so thank you for at least raising the question there), I'm still on the opposite side of the planet, so any decision made in less than 24 hours from proposal to resolution is necessarily "too fast" for global collaboration (and 48 hours is better). I'm not particularly fond of __definition_order__ either, but one of the specific points raised in the PEP-520 discussions due to Inada-san's pending patch was whether or not making class namespaces ordered by default would eliminate the need for the attribute, and the conclusion was that it *wouldn't*. So the "We made class namespaces ordered, so now __definition_order__ is gone" sounded a lot like folks forgetting that part of the discussions, and instead getting rid of a feature that was there for multiple reasons, not just because __dict__ didn't preserve the order. |
All reasonable points. You just sounded upset and I wanted to point out the decision was not made lightly, without discussion with the person in charge of the proposal and the BDFL, or we were breaking backwards-compatibility due to some drunken ordered dictionary stupor we were in. :) Anyway, that's it and any further comments I have on this specific topic I'll put on the python-dev thread. |
What's the status of this issue? It's still marked as a "deferred blocker" for 3.6. |
Thanks for the ping Ned. Closing this again, as Guido explained the rationale for the change in the python-dev thread referenced above: https://mail.python.org/pipermail/python-dev/2016-September/146371.html |
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: