Author eric.snow
Recipients eric.snow, rhettinger, serhiy.storchaka
Date 2015-10-17.16:01:19
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1445097680.66.0.821868605697.issue25410@psf.upfronthosting.co.za>
In-reply-to
Content
Regarding Py_TYPE(od) vs. od.__class__, there is a difference for subclasses, as you demonstrated in your example. [1]  Thanks for explaining your rationale.  I now understand your argument about using PyTYPE() for repr and pickle in C types.  I still have concerns, though, regarding parity between the two OrderedDict implementations.

The key difference is that we're talking about an after-the-fact C port of an existing pure Python implementation.  The two implementations should behave identically in nearly every case.  The only place I would expect a deviation to not matter is for anything where Python-as-a-whole does not guarantee behavior.  However there are *very* few of those when all is said and done.  Any other variation should not be made casually and only if we are willing to accept that there may be code out there that relies on the pure Python behavior which the C implementation will break.

So like I said before, as a rule I'm absolutely okay with changing the behavior as long as the pure Python implementation is changed to match and OrderedDict remains backward-compatible (and the change is meaningful, e.g. efficiency, consistency).  Otherwise my concerns remain and we have to have sufficient justification for the change.

It may be worth taking this to python-dev to get a clearer consensus on both "Py_TYPE(obj) vs. obj.__class__", as well as about parity between dual pure-Python/C implementations in the stdlib, regardless of the outcome of this issue.  Both are points about which we should be consistent throughout Python.  The type() vs. __class__ question may deserve an entry in the language reference and both may deserve a PEP.  

-----

For this particular case, I think we should still aim for compatibility with the pure Python implementation.  To that effect, we could use Py_TYPE(od) only if PyODict_CheckExact() returns true (as a fast path) and use od.__class__ otherwise.  That fast path would be safe for the C implementation since code can't change OrderedDict().__class__ (due to #24912).

That particular difference in the implementations (i.e. you *can* change od.__class__ for the pure Python one) is an acceptable compatibility break since it's unlikely anyone is changing od.__class__ *and* if they are then they can just switch to a simple subclass that wraps OrderedDict:

    # before:
    from collections import OrderedDict
    od = OrderedDict()
    od.__class__ = SomethingElse

    # after:
    import collections
    class OrderedDict(collections.OrderedDict):
        pass
    od = OrderedDict()
    od.__class__ = SomethingElse

If we *do* continue supporting "type(od) != od.__class__" in repr then I'd say why bother with a fast path for PyOdict_CheckExact().  That sort of efficiency isn't necessary for repr.  If we stop supporting a differing od.__class__ then I'm fine simply using Py_TYPE() throughout.
    
Likewise, if this is not a case we want to support then we must accept that we may break some code out there, however unlikely that is.  In that case perhaps we could be more clear in the documentation that OrderedDict().__class__ should not be changed, though such an addition to the OrderedDict docs might just be clutter.  A general FAQ or other broader doc entry about not assigning to obj.__class__ for stdlib types might be more appropriate.  But that is where clarification from python-dev would help.


[1] There is also a difference between type(obj) and obj.__class__ in the case of proxies (e.g. see #16251), but that is less of an issue here.
History
Date User Action Args
2015-10-17 16:01:20eric.snowsetrecipients: + eric.snow, rhettinger, serhiy.storchaka
2015-10-17 16:01:20eric.snowsetmessageid: <1445097680.66.0.821868605697.issue25410@psf.upfronthosting.co.za>
2015-10-17 16:01:20eric.snowlinkissue25410 messages
2015-10-17 16:01:19eric.snowcreate