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. <wink>

> 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.

--Guido van Rossum (python.org/~guido)