classification
Title: Make class definition namespace ordered by default
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: 16991 Superseder:
Assigned To: eric.snow Nosy List: berker.peksag, eric.snow, ethan.furman, fijall, jcea, ncoghlan, ned.deily, python-dev, vstinner
Priority: deferred blocker Keywords: patch

Created on 2015-05-21 05:34 by eric.snow, last changed 2017-05-17 18:21 by jcea. This issue is now closed.

Files
File name Uploaded Description Edit
odict-class-definition-namespace.diff eric.snow, 2015-05-21 05:34
just-default-to-odict.diff eric.snow, 2016-05-13 00:26 review
deforder.diff eric.snow, 2016-06-03 23:09 review
deforder-with-tp-slot.diff eric.snow, 2016-07-15 21:57 review
Messages (35)
msg243729 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-05-21 05:34
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 #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.
msg243767 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-05-21 17:51
Here's a patch that drops adding __definition_order__.  You can get the same effect by adding `__definition_order__ = list(locals())` at the bottom of your class definition.  The benefit of having `__definition_order__` is that the information is automatically available for later use rather than forcing the class author to add the above line in order to benefit from external introspection/manipulation.  The downside is that every syntax-defined class will have the extra attribute with the list of definition order, which might be an unnecessary waste, even if not a huge one.

Ultimately, I'd prefer to keep `__definition_order__`.
msg243902 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-05-23 09:10
types.prepare_class() also needs to be updated to use OrderedDict() by default.
msg244055 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-05-25 22:44
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.
msg244056 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-05-25 22:48
> It may be a stretch to get this into 3.5, but the final change should be pretty small.

Changing the default type of class dictionaries is a huge change. IMO it should be deferred to Python 3.6.
msg244060 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-05-25 22:51
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 `type.__prepare__`.  cls.__dict__ will remain a dict.
msg244061 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-05-25 22:52
This is *not* about changing the default type of class dictionaries (which
I agree would be far too large a change to make without a PEP), it's only
about changing the ephemeral evaluation namespace used to execute the class
body.
msg244075 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-05-26 01:34
Thanks for pointing out types.prepare_class.  I've updated it.
msg244077 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-05-26 02:03
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. :)
msg265434 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2016-05-13 00:26
Here's a refresh of the patch that only sets the default definition namespace (and does not introduce __definition_order__).
msg266957 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-06-02 21:23
Patch review sent. The motivation for the change is relatively weak without __definition_order__, though :)
msg266981 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2016-06-02 22:57
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.
msg267183 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2016-06-03 22:55
Here's the full patch, including the addition of __definition_order__, tests, and docs.
msg267319 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-06-04 21:11
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.
msg269846 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2016-07-05 19:56
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.
msg270431 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2016-07-14 17:50
FTR: PEP 520 "Preserving Class Attribute Definition Order" (https://www.python.org/dev/peps/pep-0520/)
msg270517 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2016-07-15 21:57
post-review updates
msg273881 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-08-29 20:32
Any update on this? b1 is exactly 2 weeks away at this point.
msg273887 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-08-29 21:23
Brett Cannon added the comment:
> Any update on this? b1 is exactly 2 weeks away at this point.

Why do we need changes specific to classes, if dictionaries could be ordered by default?

Issue #27350: "Compact and ordered dict" by INADA Naoki
msg273898 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-08-30 03:10
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.
msg274441 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-09-05 21:53
New changeset 635fd3912d4d by Eric Snow in branch 'default':
Issue #24254: Preserve class attribute definition order.
https://hg.python.org/cpython/rev/635fd3912d4d
msg275188 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-09-08 22:31
New changeset 9aa5424fd1df by Eric Snow in branch 'default':
Issue #24254: Drop cls.__definition_order__.
https://hg.python.org/cpython/rev/9aa5424fd1df
msg275337 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2016-09-09 16:42
Is this going to be added back?  Should I add __definition_order__ to Enum?
msg275347 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-09-09 16:54
Nope, PEP 520 has been updated to drop __definition_order__ and instead state that cls.__dict__ is an ordered mapping.
msg275580 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2016-09-10 06:46
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.
msg275585 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-09-10 06:55
You should reopen the discussion on python-dev, since the PEP 520 has
been accepted with:

"Note: Since compact dict has landed in 3.6, __definition_order__ has
been removed. cls.__dict__ now mostly accomplishes the same thing
instead."

The PEP status is now Final.
msg275605 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-09-10 09:24
The danger of sprints, and decisions being made without adequate input from affected parties.
msg275606 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-09-10 09:29
Ethan started a python-dev thread here: https://mail.python.org/pipermail/python-dev/2016-September/146358.html
msg275622 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-09-10 11:11
Reopening until the __definition_order__ question has been resolved.
msg275624 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-09-10 11:13
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.
msg275656 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-09-10 17:03
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.
msg275660 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-09-10 17:24
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.
msg275662 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-09-10 17:38
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.
msg282574 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2016-12-06 22:46
What's the status of this issue?  It's still marked as a "deferred blocker" for 3.6.
msg282583 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-12-07 00:56
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
History
Date User Action Args
2017-05-17 18:21:22jceasetnosy: + jcea
2016-12-07 00:56:52ncoghlansetstatus: open -> closed
resolution: fixed
messages: + msg282583

stage: commit review -> resolved
2016-12-06 22:46:58ned.deilysetnosy: + ned.deily
messages: + msg282574
2016-09-10 17:38:45brett.cannonsetnosy: - brett.cannon
2016-09-10 17:38:30brett.cannonsetmessages: + msg275662
2016-09-10 17:24:54ncoghlansetmessages: + msg275660
2016-09-10 17:03:27brett.cannonsetmessages: + msg275656
2016-09-10 11:13:17ncoghlansetpriority: deferred blocker

messages: + msg275624
2016-09-10 11:11:31ncoghlansetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg275622

stage: resolved -> commit review
2016-09-10 09:29:11ncoghlansetmessages: + msg275606
2016-09-10 09:24:13ncoghlansetmessages: + msg275605
2016-09-10 06:55:48vstinnersetmessages: + msg275585
2016-09-10 06:46:28ethan.furmansetmessages: + msg275580
2016-09-09 16:54:02brett.cannonsetmessages: + msg275347
2016-09-09 16:42:00ethan.furmansetmessages: + msg275337
2016-09-08 22:36:54ethan.furmansetnosy: + ethan.furman
2016-09-08 22:32:37eric.snowsetmessages: - msg275185
2016-09-08 22:31:46python-devsetmessages: + msg275188
2016-09-08 22:25:12eric.snowsetmessages: + msg275185
2016-09-05 22:25:51eric.snowsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2016-09-05 21:53:52python-devsetnosy: + python-dev
messages: + msg274441
2016-08-30 03:10:49ncoghlansetmessages: + msg273898
2016-08-29 21:24:21vstinnersetnosy: + fijall
2016-08-29 21:23:49vstinnersetmessages: + msg273887
2016-08-29 20:32:29brett.cannonsetnosy: + brett.cannon
messages: + msg273881
2016-07-19 22:26:35eric.snowsethgrepos: - hgrepo310
2016-07-19 22:26:27eric.snowsetfiles: - deforder-with-tp-slot.diff
2016-07-15 21:57:49eric.snowsetfiles: + deforder-with-tp-slot.diff

messages: + msg270517
2016-07-14 17:50:08eric.snowsetmessages: + msg270431
2016-07-05 19:56:20eric.snowsetfiles: + deforder-with-tp-slot.diff

messages: + msg269846
2016-06-13 18:22:31berker.peksagsetnosy: + berker.peksag
2016-06-04 21:11:37ncoghlansetmessages: + msg267319
2016-06-03 23:09:41eric.snowsetfiles: + deforder.diff
2016-06-03 23:09:25eric.snowsetfiles: - deforder.diff
2016-06-03 22:55:44eric.snowsetfiles: + deforder.diff

messages: + msg267183
2016-06-02 22:57:05eric.snowsetmessages: + msg266981
2016-06-02 21:23:31ncoghlansetmessages: + msg266957
2016-05-13 00:28:28eric.snowsetfiles: - just-default-to-odict.diff
2016-05-13 00:26:59eric.snowsetfiles: + just-default-to-odict.diff

messages: + msg265434
2015-05-26 02:03:17eric.snowsetmessages: + msg244077
2015-05-26 02:00:57eric.snowsetpriority: release blocker -> (no value)
stage: patch review
versions: + Python 3.6, - Python 3.5
2015-05-26 01:34:06eric.snowsetmessages: + msg244075
2015-05-25 22:52:31ncoghlansetmessages: + msg244061
2015-05-25 22:51:55eric.snowsetmessages: + msg244060
2015-05-25 22:48:22vstinnersetnosy: + vstinner
messages: + msg244056
2015-05-25 22:44:52eric.snowsetpriority: normal -> release blocker
2015-05-25 22:44:42eric.snowsetmessages: + msg244055
2015-05-24 02:17:57eric.snowsethgrepos: + hgrepo310
2015-05-23 09:10:00ncoghlansetdependencies: + Add OrderedDict written in C
messages: + msg243902
2015-05-21 17:51:38eric.snowsetfiles: + just-default-to-odict.diff

messages: + msg243767
2015-05-21 05:34:48eric.snowcreate