msg243729 - (view) |
Author: Eric Snow (eric.snow) *  |
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) *  |
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: Alyssa Coghlan (ncoghlan) *  |
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) *  |
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) *  |
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) *  |
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: Alyssa Coghlan (ncoghlan) *  |
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) *  |
Date: 2015-05-26 01:34 |
Thanks for pointing out types.prepare_class. I've updated it.
|
msg244077 - (view) |
Author: Eric Snow (eric.snow) *  |
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) *  |
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: Alyssa Coghlan (ncoghlan) *  |
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) *  |
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) *  |
Date: 2016-06-03 22:55 |
Here's the full patch, including the addition of __definition_order__, tests, and docs.
|
msg267319 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
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) *  |
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) *  |
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) *  |
Date: 2016-07-15 21:57 |
post-review updates
|
msg273881 - (view) |
Author: Brett Cannon (brett.cannon) *  |
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) *  |
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: Alyssa Coghlan (ncoghlan) *  |
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)  |
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)  |
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) *  |
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) *  |
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) *  |
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) *  |
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: Alyssa Coghlan (ncoghlan) *  |
Date: 2016-09-10 09:24 |
The danger of sprints, and decisions being made without adequate input from affected parties.
|
msg275606 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
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: Alyssa Coghlan (ncoghlan) *  |
Date: 2016-09-10 11:11 |
Reopening until the __definition_order__ question has been resolved.
|
msg275624 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
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) *  |
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: Alyssa Coghlan (ncoghlan) *  |
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) *  |
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) *  |
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: Alyssa Coghlan (ncoghlan) *  |
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
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:17 | admin | set | github: 68442 |
2017-05-17 18:21:22 | jcea | set | nosy:
+ jcea
|
2016-12-07 00:56:52 | ncoghlan | set | status: open -> closed resolution: fixed messages:
+ msg282583
stage: commit review -> resolved |
2016-12-06 22:46:58 | ned.deily | set | nosy:
+ ned.deily messages:
+ msg282574
|
2016-09-10 17:38:45 | brett.cannon | set | nosy:
- brett.cannon
|
2016-09-10 17:38:30 | brett.cannon | set | messages:
+ msg275662 |
2016-09-10 17:24:54 | ncoghlan | set | messages:
+ msg275660 |
2016-09-10 17:03:27 | brett.cannon | set | messages:
+ msg275656 |
2016-09-10 11:13:17 | ncoghlan | set | priority: deferred blocker
messages:
+ msg275624 |
2016-09-10 11:11:31 | ncoghlan | set | status: closed -> open resolution: fixed -> (no value) messages:
+ msg275622
stage: resolved -> commit review |
2016-09-10 09:29:11 | ncoghlan | set | messages:
+ msg275606 |
2016-09-10 09:24:13 | ncoghlan | set | messages:
+ msg275605 |
2016-09-10 06:55:48 | vstinner | set | messages:
+ msg275585 |
2016-09-10 06:46:28 | ethan.furman | set | messages:
+ msg275580 |
2016-09-09 16:54:02 | brett.cannon | set | messages:
+ msg275347 |
2016-09-09 16:42:00 | ethan.furman | set | messages:
+ msg275337 |
2016-09-08 22:36:54 | ethan.furman | set | nosy:
+ ethan.furman
|
2016-09-08 22:32:37 | eric.snow | set | messages:
- msg275185 |
2016-09-08 22:31:46 | python-dev | set | messages:
+ msg275188 |
2016-09-08 22:25:12 | eric.snow | set | messages:
+ msg275185 |
2016-09-05 22:25:51 | eric.snow | set | status: open -> closed resolution: fixed stage: patch review -> resolved |
2016-09-05 21:53:52 | python-dev | set | nosy:
+ python-dev messages:
+ msg274441
|
2016-08-30 03:10:49 | ncoghlan | set | messages:
+ msg273898 |
2016-08-29 21:24:21 | vstinner | set | nosy:
+ fijall
|
2016-08-29 21:23:49 | vstinner | set | messages:
+ msg273887 |
2016-08-29 20:32:29 | brett.cannon | set | nosy:
+ brett.cannon messages:
+ msg273881
|
2016-07-19 22:26:35 | eric.snow | set | hgrepos:
- hgrepo310 |
2016-07-19 22:26:27 | eric.snow | set | files:
- deforder-with-tp-slot.diff |
2016-07-15 21:57:49 | eric.snow | set | files:
+ deforder-with-tp-slot.diff
messages:
+ msg270517 |
2016-07-14 17:50:08 | eric.snow | set | messages:
+ msg270431 |
2016-07-05 19:56:20 | eric.snow | set | files:
+ deforder-with-tp-slot.diff
messages:
+ msg269846 |
2016-06-13 18:22:31 | berker.peksag | set | nosy:
+ berker.peksag
|
2016-06-04 21:11:37 | ncoghlan | set | messages:
+ msg267319 |
2016-06-03 23:09:41 | eric.snow | set | files:
+ deforder.diff |
2016-06-03 23:09:25 | eric.snow | set | files:
- deforder.diff |
2016-06-03 22:55:44 | eric.snow | set | files:
+ deforder.diff
messages:
+ msg267183 |
2016-06-02 22:57:05 | eric.snow | set | messages:
+ msg266981 |
2016-06-02 21:23:31 | ncoghlan | set | messages:
+ msg266957 |
2016-05-13 00:28:28 | eric.snow | set | files:
- just-default-to-odict.diff |
2016-05-13 00:26:59 | eric.snow | set | files:
+ just-default-to-odict.diff
messages:
+ msg265434 |
2015-05-26 02:03:17 | eric.snow | set | messages:
+ msg244077 |
2015-05-26 02:00:57 | eric.snow | set | priority: release blocker -> (no value) stage: patch review versions:
+ Python 3.6, - Python 3.5 |
2015-05-26 01:34:06 | eric.snow | set | messages:
+ msg244075 |
2015-05-25 22:52:31 | ncoghlan | set | messages:
+ msg244061 |
2015-05-25 22:51:55 | eric.snow | set | messages:
+ msg244060 |
2015-05-25 22:48:22 | vstinner | set | nosy:
+ vstinner messages:
+ msg244056
|
2015-05-25 22:44:52 | eric.snow | set | priority: normal -> release blocker |
2015-05-25 22:44:42 | eric.snow | set | messages:
+ msg244055 |
2015-05-24 02:17:57 | eric.snow | set | hgrepos:
+ hgrepo310 |
2015-05-23 09:10:00 | ncoghlan | set | dependencies:
+ Add OrderedDict written in C messages:
+ msg243902 |
2015-05-21 17:51:38 | eric.snow | set | files:
+ just-default-to-odict.diff
messages:
+ msg243767 |
2015-05-21 05:34:48 | eric.snow | create | |