Ethan Furman added the comment: Ethan stated: >> The only problem with the design is that it does not play well with others. >> For >> duck-typeable just do a check on 'other' to see if it has an .items() >> method, and >> return NotImplemented if it does not. Oh, and check that 'self' is Counter, >> and >> also return NotImplemented if that fails.
R. David Murray replied: > No, that doesn't support duck typing. Duck typing is not putting arbitrary > restrictions on what objects you accept, but only essential restrictions. __iadd__ will raise an AttributeError if 'other' doesn't have an 'items' method -- how is that an arbitrary restriction? If 'other' doesn't have an 'items' but does know how to add itself to Counter, it won't matter because Counter is raising an exception instead of returning NotImplemented. What other types return NotImplemented when 'other' is not the expected (sub)type? bytes, bytearray, int, float, string, dict, list, tuple, Enum, deque, defaultdict, OrderedDict, Decimal, Fraction, and Counter itself for every binary method /except/ the four inplace ops it supplies itself. > And as noted above, if you return NotImplemented, then you get an error > message > that essentially says "this is impossible" instead of "this is what didn't > work". Correct implementation should not be sacrificed for a nicer error message. >> Here's proof of subclass trouble -- the idea is to make an immutable Counter: >> [snip example] > Your subclass doesn't override __iadd__ or any of the other mutation methods. > Why > would you expect it to be immutable if it doesn't? You're right, that was a bad example. A subclass would need to override any method whose behavior it wants to change. What does not seem correct to me is raising an exception because the type of 'other' is not correct, instead of returning NotImplemented, and in 14 other core datatypes it does work that way, and even in Counter it works that way for any in-place method that Counter itself doesn't override, and it does work that way /in/ Counter for the normal binary ops that it /does/ override. > For subclassing to work, the fix is: > > if not hasattr(other, 'items') or type(self) is not Counter: > return NotImplemented >> I've never seen a type check like that in a Python class, and I hope I never >> do. *That* >> breaks subclassing. Explanations and examples go a lot further than bare statements. [Jim Jewitt posts an explanation.] Ah, I see. Thank you, Jim. Okay, I agree that the check against the base class is ill-advised (and I modified some of the above reply to match that understanding). However, I am still unconvinced that 'other' should not be checked. Hopefully the python-dev thread will shed more light on this. ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue22766> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
