(original) (raw)
On Fri, Jun 24, 2016 at 3:46 PM, Eric Snow <ericsnowcurrently@gmail.com> wrote:
On Fri, Jun 24, 2016 at 4:37 PM, Nick Coghlan <ncoghlan@gmail.com> wrote:
\> This version looks fine to me.
\\o/
Same to me, mostly.
\> The definition order question has been dropped from PEP 487, so this
\> cross-reference doesn't really make sense any more :)
Ah, so much for my appeal to authority.
\> I'd characterise this section at the language definition level as the
\> default class definition namespace now being \*permitted\* to be an
\> OrderedDict. For implementations where dict is ordered by default,
\> there's no requirement to switch specifically to
\> collections.OrderedDict.
Yeah, I'd meant to fix that.
Please do.
\> This paragraph is a little confusing, since "set
\> \`\`\_\_definition\_order\_\_\`\` manually" is ambiguous.
\>
\> "supply an explicit \`\`\_\_definition\_order\_\_\`\` via the class namespace"
\> might be clearer.
ack
\> I realised there's another important reason for doing it this way by
\> default: it's \*really easy\* to write a "skip\_dunder\_names" filter that
\> leaves out dunder names from an arbitrary interable of strings. It's
\> flatout \*impossible\* to restore the dunder attribute order if the
\> class definition process throws it away.
Yep. That's why I felt fine with relaxing that. I guess I didn't
actually put that in the PEP though. :)
Please add it. I'd also like the PEP point out that there might be other things that an app wouldn't want in the definition order, e.g. anything that's a method, or anything that starts with '\_', etc.
I still think that it should not be read-only. If \_\_slots\_\_ and \_\_name\_\_ can be writable I think \_\_definition\_order\_\_ can be too. (I believe an easy way to make it so should be to add it to the dict that's passed to type.\_\_new\_\_().)
Other nits:
- I don't think it's great to let other implementations leave \_\_definition\_order\_\_ set to None when they don't care to support it; this would break apps/libraries that want to use it and the implementation could refuse to fix it, claiming PEP 520 doesn't mandate it. I think it's better to mandate this from a confirming implementation.
- I don't think there's much of a use case for setting \_\_definition\_order\_\_ (to a tuple) for builtin classes. However I do think extension modules should be allowed to set it, in case they are substituting for a previous Python-level class whose users expect this to work.