[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Change by Raymond Hettinger : -- status: pending -> closed ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Alex Waygood added the comment: The proposed patch appears to have been implemented in https://github.com/python/cpython/commit/97c1adf3935234da716d3289b85f72dcd67e90c2, and there has been no discussion for five years. I think this can now be closed. -- nosy: +AlexWaygood

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Andrew Barnert added the comment: > My main concern about the documentation was that in your patch you say _all_ > special methods are now allowed to be None, but in your code you only check > __iter__(). This is adding new undocumented inconsistencies e.g. with > Iterable vs Container. No

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Martin Panter added the comment: I’m sorry I only read your patch and did not see the NoLen class above Blocked. (I blame the lack of Reitveld review link.) I still think __getitem__() should have a valid signature, but I acknowledge that it’s not really your fault. :) My main concern about

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Andrew Barnert added the comment: > If this patch goes ahead, I think the ABC documentation should clarify which > methods are checked for None and which aren’t. That seems fair. Also, as you pointed out on #25958, at least one other ABC has the same problem as Iterable: you can block the

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Martin Panter added the comment: IMO allowing any special method to be set to None seems to make more trouble than it is worth. Are there practical problems to address, or are they all theoretical? Ideally I think it would be better to require __reversed__() for reverse() to work, but such a

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Changes by Raymond Hettinger : -- assignee: rhettinger -> ___ Python tracker ___

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Martin Panter added the comment: If this patch goes ahead, I think the ABC documentation should clarify which methods are checked for None and which aren’t. The datamodel.rst file will suggest None for any method, but ABC will only support it for Iterable and Hashable (I think). Also, what

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Andrew Barnert added the comment: The attached patch does the following: * collections.abc.Mapping.__reversed__ = None. * collections.abc.Iterable.__subclasshook__ checks for None the same way Hashable does: * This tests for any falsey value, not just None. I'm not sure this is ideal, but

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

R. David Murray added the comment: So this issue now has two problems being discussed in it. Someone should start a new issue for the typing.Reversible problem. -- ___ Python tracker

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Andrew Barnert added the comment: > Perhaps there is a bug in typing.Reversible. It doesn't accept all types > supported by reversed(). > ... And accepts types that don't work with reversed(). The problem is the way the two are defined: * Reversible is true if you implement __reversed__ *

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Guido van Rossum added the comment: This sounds good. Also, reversed() could then be modified to produce a better error. (The "unhashable" error comes from the hash() builtin, so that's also a precedent.) On Sat, Dec 26, 2015 at 4:32 PM, Andrew Barnert wrote: > >

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Andrew Barnert added the comment: As mentioned in #25958, Guido pointed out on -ideas that `__hash__ = None` is already the standard way to declare a class unhashable, and it's recognized by `collections.abc.Hashable`. Doing `__reversed__ = None` does make `reversed(m)` raise a `TypeError`

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Andrew Barnert added the comment: > This sounds good. Also, reversed() could then be modified to produce a better error. Should `iter` also be modified to produce a better error if `__iter__` is None? Also, should this be documented? Maybe a sentence in the "Special method names" section of

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Andrew Barnert added the comment: Serhiy already filed the typing.Reversible bug on the separate typehinting tracker (https://github.com/ambv/typehinting/issues/170). So, unless fixing that bug requires some changes back to collections.abc or something else in the stdlib, I think the only

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Andrew Barnert added the comment: Also, I filed #25958 as an ABC equivalent to Serhiy's typehinting problem. I don't know if that actually needs to be solved, but that definitely takes it out of the way for this issue. -- ___ Python tracker

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Changes by Serhiy Storchaka : -- stage: -> needs patch ___ Python tracker ___ ___

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Guido van Rossum added the comment: All sounds fine. -- ___ Python tracker ___ ___ Python-bugs-list mailing

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Changes by Nick Coghlan : -- nosy: +ncoghlan ___ Python tracker ___ ___ Python-bugs-list

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Serhiy Storchaka added the comment: I agree that by default calling reversed() on mapping should raise a TypeError. But for now issubclass(collections.abc.Mapping, typing.Reversible) returns False. If add default __reversed__ implementation this test will return True. We have to find other

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

R. David Murray added the comment: You can write a patch if you like, but we haven't come to a consensus on actually doing anything. I'm leaning toward Andrew's position, but not strongly, so we need some more core dev opinions. -- ___ Python

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Terry J. Reedy added the comment: Unless this somehow breaks the philosophy of ABCs, I would be inclined to add the negative methods. -- nosy: +terry.reedy ___ Python tracker

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Swati Jaiswal added the comment: Okay, so should I go for a patch for it? And sorry if it sounds naive, but do we provide the work around or the user would implement if they purposely want it. If we provide it, then where should it be written? --

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Changes by Serhiy Storchaka : -- assignee: -> rhettinger ___ Python tracker ___ ___

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Swati Jaiswal added the comment: @Andrew Barnert, sorry, I didn't get your previous messages so please ignore the last message i sent. I got your point i.e. We just need to provide the TypeError in the Mapping. And the work around is never implemented. Should I go for the patch with it?

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Andrew Barnert added the comment: @Swati Jaiswal: > But the work around suggested here ... is also not a general solution, i.e. > ... for any other mapping which does not have 0 as a key, it results in > KeyError. You're missing the point. The workaround isn't intended to be a general

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Andrew Barnert added the comment: @R. David Murray: > What is it that makes reversed raise a typeerror on dict here? There are separate slots for tp_as_sequence and tp_as_mapping, so a C type can be (and generally is) one or the other, not both. But for Python types, anything that has

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

R. David Murray added the comment: No, the workaround was for duplicating the existing behavior if the fix (raising an error like a normal dict does) broken someone's code. The *only* possibility here is to have a __reversed__ that raises a TypeError. What is it that makes reversed raise a

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Swati Jaiswal added the comment: But the work around suggested here as: def __reversed__(self): return (self[k] for k in reversed(range(len(self is also not a general solution, i.e. it is applicable for the following case: m = MyDict({2:40, 0:10, 1:20}) but for any other mapping

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

New submission from Andrew Barnert: Example: class MyDict(collections.abc.Mapping): def __init__(self, d): self.d = d def __len__(self): return len(self.d) def __getitem__(self, key): return self.d[key] def __iter__(self): return iter(self.d) d = {1:2,

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Swati Jaiswal added the comment: > If you do `reversed(d)`, you get a nice `TypeError: argument to reversed() > must be a sequence`. But if you do `reversed(m)`, you get a reversed` > iterator. And when you iterate it, presumably expecting to get 0 and 1 in > some arbitrary order, you instead

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Andrew Barnert added the comment: > It seems no more interesting or problematic than sequence argument unpacking > working with dictionaries, "a, b = {'one': 1, 'two': 2}". Dictionaries (including dicts, dict subclasses, and custom Mappings) are iterables. People use that fact every time they

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Swati Jaiswal added the comment: Can it be reproduced in default branch? I tried but got: AttributeError: module 'collections' has no attribute 'abc' -- nosy: +curioswati ___ Python tracker

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Raymond Hettinger added the comment: I've seen no evidence that this a problem in practice. It seems no more interesting or problematic than sequence argument unpacking working with dictionaries, "a, b = {'one': 1, 'two': 2}". -- nosy: +rhettinger

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Emanuel Barry added the comment: You need to do 'import collections.abc' as abc is a submodule of collections, and is not imported from a bare 'import collections'. -- nosy: +ebarry ___ Python tracker

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Changes by Raymond Hettinger : -- priority: normal -> low versions: +Python 3.6 ___ Python tracker ___

[issue25864] collections.abc.Mapping should include a __reversed__ that raises TypeError

Andrew Barnert added the comment: > What are we exactly expecting here? Well, naively, I was expecting a TypeError, just as you get for dict, or a subclass of dict, or a custom extension type that implements the C-API mapping protocol. Once you understand how reversed works, you can