[issue35456] asyncio.Task.set_result() and set_exception() missing docstrings (and Liskov sub. principle)
New submission from Yahya Abou Imran : In asyncio.Task help: | set_exception(self, exception, /) | Mark the future done and set an exception. | | If the future is already done when this method is called, raises | InvalidStateError. | | set_result(self, result, /) | Mark the future done and set its result. | | If the future is already done when this method is called, raises | InvalidStateError. These doctrings are inherited from asyncio.Future. But in fact it's wrong since: https://github.com/python/cpython/blob/4824385fec0a1de99b4183f995a3e4923771bf64/Lib/asyncio/tasks.py#L161: def set_result(self, result): raise RuntimeError('Task does not support set_result operation') def set_exception(self, exception): raise RuntimeError('Task does not support set_exception operation') Just adding another docstring is not a good solution - at leas for me - because the problem is in fact deeper: This prove by itself that a Task is not a Future in fact, or shouldn't be, because this breaks the Liskov substitution principle. We could have both Future and Task inheriting from some base class like PendingOperation witch would contain all the methods of Future except these two setters. One problem to deal with might be those calls to super().set_result/exception() in Task._step(): https://github.com/python/cpython/blob/4824385fec0a1de99b4183f995a3e4923771bf64/Lib/asyncio/tasks.py#L254 except StopIteration as exc: if self._must_cancel: # Task is cancelled right before coro stops. self._must_cancel = False super().set_exception(exceptions.CancelledError()) else: super().set_result(exc.value) except exceptions.CancelledError: super().cancel() # I.e., Future.cancel(self). except Exception as exc: super().set_exception(exc) except BaseException as exc: super().set_exception(exc) raise One way to deal with that would be to let a Task have a Future. "Prefer composition over inheritance" as they say. I want to work on PR for this if nobody goes against it... PS: I really don't like when some people says that Python core developers are known to have poor knowledge in regard to OOP principles. So I really don't like letting something like this in the standard library... -- messages: 331570 nosy: yahya-abou-imran priority: normal severity: normal status: open title: asyncio.Task.set_result() and set_exception() missing docstrings (and Liskov sub. principle) type: enhancement versions: Python 3.8 ___ Python tracker <https://bugs.python.org/issue35456> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32471] Add an UML class diagram to the collections.abc module documentation
Yahya Abou Imran added the comment: I succeed in submitting a PR and building the doc locally. But there is a little problem of consistency with the abstract methods: in some classes, the inherited one are mentioned (Collection, Sequence, Mapping), but not in some others (Coroutine, Reversible). So I don't not what to show and what to hide. I opened an other issue for this: https://bugs.python.org/issue32621 -- ___ Python tracker <https://bugs.python.org/issue32471> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32621] Problem of consistence in collection.abc documentation
Yahya Abou Imran added the comment: There is another one: Reversible list __reversed__ in the abstract method but inherits __iter__ from Iterable. So there is definitely an inconsistency... -- ___ Python tracker <https://bugs.python.org/issue32621> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32621] Problem of consistence in collection.abc documentation
New submission from Yahya Abou Imran : Opened after https://github.com/python/cpython/pull/5270 was closed. Here: https://docs.python.org/3/library/collections.abc.html Some abstract methods are inherited from a superclass. Most of the time the name of the method is mentioned in the subclass. For example: Collection inherit from Sized, Iterable and Contains. But __len__, __iter__ and __contains__ are mentioned, even if they are inherited. Mapping inherits from Collection, but __len__ and __iter__ appears in the table There is one exception: Coroutine. It inherits from Awaitable but we don't see __await__. What would we do? Let all appear or not? -- messages: 310422 nosy: yahya-abou-imran priority: normal severity: normal status: open title: Problem of consistence in collection.abc documentation type: enhancement versions: Python 3.6, Python 3.7 ___ Python tracker <https://bugs.python.org/issue32621> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32472] Mention of __await__ missing in Coroutine Abstract Methods
Change by Yahya Abou Imran : -- keywords: +patch pull_requests: +5114 stage: -> patch review ___ Python tracker <https://bugs.python.org/issue32472> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32467] dict_values isn't considered a Collection nor a Container
Yahya Abou Imran added the comment: Perfect for me. I was about to submit about the same patch as yours after your acceptation; but it's better that way, it's your module! Thanks Raymond. -- ___ Python tracker <https://bugs.python.org/issue32467> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32471] Add an UML class diagram to the collections.abc module documentation
Change by Yahya Abou Imran : -- keywords: +patch pull_requests: +4994 stage: -> patch review ___ Python tracker <https://bugs.python.org/issue32471> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32471] Add an UML class diagram to the collections.abc module documentation
Yahya Abou Imran added the comment: I have generated SVG files. Here is the rest of the ABCs present in the module: https://gitlab.com/yahya-abou-imran/collections-abc-uml/blob/master/plantuml/other_collections.svg And here is the full version with all ABCs: https://gitlab.com/yahya-abou-imran/collections-abc-uml/blob/master/plantuml/full.svg What do you prefer? I'm gonna submit a PR with the full version right now. I will correct it if you think it's too big. But I don't know how to run the server of the documentation locally, so I can't test it to see... -- ___ Python tracker <https://bugs.python.org/issue32471> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32449] MappingView must inherit from Collection instead of Sized
Yahya Abou Imran added the comment: I'm sorry, It's already done... https://bugs.python.org/issue32467 -- ___ Python tracker <https://bugs.python.org/issue32449> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32473] Readibility of ABCMeta._dump_registry()
Change by Yahya Abou Imran : -- keywords: +patch pull_requests: +4963 stage: -> patch review ___ Python tracker <https://bugs.python.org/issue32473> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32473] Readibility of ABCMeta._dump_registry()
New submission from Yahya Abou Imran : >From python-ideas: https://mail.python.org/pipermail/python-ideas/2017-December/048504.html In python 2.7, ABCs's caches and registries are sets. But in python 3.6 they are WeakSet. In consequence, the output of _dump_registry() is almost useless: >>> from collections import abc >>> abc.Iterator._dump_registry() Class: collections.abc.Iterator Inv.counter: 40 _abc_cache: <_weakrefset.WeakSet object at 0x7f4b58fe2668> _abc_negative_cache: <_weakrefset.WeakSet object at 0x7f4b53283780> _abc_negative_cache_version: 40 _abc_registry: <_weakrefset.WeakSet object at 0x7f4b58fe2630> We could convert them into a regular set before printing: if isinstance(value, WeakSet): value = set(value) The result: >>> abc.Iterator._dump_registry() Class: collections.abc.Iterator Inv.counter: 40 _abc_cache: {, , , , , , , , , , , , } _abc_negative_cache: set() _abc_negative_cache_version: 40 _abc_registry: set() -- messages: 309321 nosy: yahya-abou-imran priority: normal severity: normal status: open title: Readibility of ABCMeta._dump_registry() type: enhancement versions: Python 3.6, Python 3.7 ___ Python tracker <https://bugs.python.org/issue32473> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32472] Mention of __await__ missing in Coroutine Abstract Methods
New submission from Yahya Abou Imran : In the collections.abc documentation: https://docs.python.org/3/library/collections.abc.html __await__() doesn't appear in the abstract methods of Coroutine, we see only send() and throw(). But since Coroutine inherit from Awaitable, it's required: from collections.abc import Coroutine class MyCoroutine(Coroutine): def send(self, value): raise StopIteration def throw(self, err): raise err mc = MyCoroutine() Traceback (most recent call last): File "_tmp.py", line 9, in mc = MyCoroutine() TypeError: Can't instantiate abstract class MyCoroutine with abstract methods __await__ To be consistent with the rest of the document, this method should appear here to show all the abstract methods, even the inherited ones. -- assignee: docs@python components: Documentation messages: 309320 nosy: docs@python, yahya-abou-imran priority: normal severity: normal status: open title: Mention of __await__ missing in Coroutine Abstract Methods type: enhancement versions: Python 3.6, Python 3.7 ___ Python tracker <https://bugs.python.org/issue32472> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32471] Add an UML class diagram to the collections.abc module documentation
Yahya Abou Imran added the comment: Here is the .puml -- Added file: https://bugs.python.org/file47358/base.puml ___ Python tracker <https://bugs.python.org/issue32471> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32471] Add an UML class diagram to the collections.abc module documentation
New submission from Yahya Abou Imran : >From python-ideas: >https://mail.python.org/pipermail/python-ideas/2017-December/048492.html In this page of the documentation: https://docs.python.org/3/library/collections.abc.html The table could be difficult to understand, a diagram help visualize things. I'm joining the last version of my work, and the .puml I used to generate it. Opinions about details (fonts, displaying...) are being discussed on the mailist right now. -- assignee: docs@python components: Documentation files: base.png messages: 309318 nosy: docs@python, yahya-abou-imran priority: normal severity: normal status: open title: Add an UML class diagram to the collections.abc module documentation type: enhancement versions: Python 3.6, Python 3.7 Added file: https://bugs.python.org/file47357/base.png ___ Python tracker <https://bugs.python.org/issue32471> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32467] dict_values isn't considered a Collection nor a Container
New submission from Yahya Abou Imran : a `dict_values` instance behaves like a Collection (a Sized Iterable Container): >>> values = {1: 'one', 2: 'two'}.values() >>> 'two' in values True >>> 'three' in values False >>> for value in values: print(value) one two >>> len(values) 2 But... >>> isinstance(values, abc.Collection) False >>> isinstance(values, abc.Container) False The reason is the lack of __contains__(): >>> '__contains__' in dir(values) False The 'in' operator works above because it uses __iter__() to check the presence of the value. I think it's inconsistent with the fact that dict_values is in the registry of ValuesView witch passes the test: >>> issubclass(abc.ValuesView, abc.Collection) True A class passed in the registry of ValuesView should guarantee this behaviour (witch is the case here by the way, but informally). I can think about several solutions for this: 1. add a __contains__() to the dict_values class; 2. if an ABC is considered a subclass of another ABC, all the classes in the registry of the first (and recursively all of their subclasses) should be too; 3. pass dict_values in the registry of Container or Collection; 4. make ValuesView inherit from Container or Collection. IMHO: 1 is out. 2 makes sense, but may be difficult to implement since it implies that a class has to know all the registries it's been passed in. 3 and 4 are by far the easiest ways to do it. I think the inheritance from Collection is the best solution, because KeysView and ItemsView are already Collections by inheriting from Set witch inherits from Collection. The only fondamental difference between the three views (in terms of protocol and API), it's that ValuesView is not a Set. -- messages: 309289 nosy: yahya-abou-imran priority: normal severity: normal status: open title: dict_values isn't considered a Collection nor a Container type: behavior versions: Python 3.6, Python 3.7 ___ Python tracker <https://bugs.python.org/issue32467> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32449] MappingView must inherit from Collection instead of Sized
Yahya Abou Imran added the comment: Hmm... Okay, I understand. So the only objection I have left, it's that ValuesView isn't passing the is instance of Collection test whereas it should, since he has the full behavior of one. It could be passed in its register to solve this. But it's not realy the same issue. Maybe I have to open a new one? -- ___ Python tracker <https://bugs.python.org/issue32449> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32449] MappingView must inherit from Collection instead of Sized
Yahya Abou Imran added the comment: And I just saw that Raymon Hettinger made it inherting from Sized: https://github.com/python/cpython/blame/master/Lib/_collections_abc.py#L694 So I we were to ahve his opinion on this... -- ___ Python tracker <https://bugs.python.org/issue32449> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32449] MappingView must inherit from Collection instead of Sized
Yahya Abou Imran added the comment: https://mail.python.org/pipermail/python-ideas/2017-December/048489.html Guido is waiting for objection on this... I have absolutly no idea why this decision was made. These tests (ttps://github.com/python/cpython/blame/master/Lib/test/test_collections.py#L794): non_col_iterables = [_test_gen(), iter(b''), iter(bytearray()), (x for x in []), dict().values()] for x in non_col_iterables: self.assertNotIsInstance(x, Collection) were written by Guido himself. -- ___ Python tracker <https://bugs.python.org/issue32449> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32449] MappingView must inherit from Collection instead of Sized
Yahya Abou Imran added the comment: Quoting my own post on python-ideas: After I generate an UML diagram from collections.abc, I found very strange that MappingView inherit from Sized instead of Collection (new in python 3.6). Yes, MappingView only define __len__ and not __iter__ and __contains__, but all of its subclasses define them (KeysView, ValuesView and ItemViews). I tried to run the tests in test/test_collections.py after making this change and on only one fail : Traceback (most recent call last): File "/usr/lib/python3.6/test/test_collections.py", line 789, in test_Collection self.assertNotIsInstance(x, Collection) AssertionError: dict_values([]) is an instance of Wich is absolutely wrong, since in reality a dict_values instance has the behaviour of a Collection: >>> vals = {1:'a', 2: 'b'}.values() >>> 'a' in vals True >>> 'c' in vals False >>> len(vals) 2 >>> for val in vals: ... print(val) ... a b The only lack is that it doesn't define a __contains__ method: >>> '__contains__' in vals False It uses __iter__ to find the presence of the value. But, hey: we have register() for this cases! In fact, when MappingView inherit from Collection, dict_values is considered as a subclass of Collection since it's in the register of ValuesView, causing the above bug... So, the test have to be changed, and dict_values must be placed in the samples that pass the test, and not in the ones that fail it. Here is a patch file for this. The only problem in this is that the MappingView won't be instatiable anymore because of the lack of __contains__ and __iter__. But since - if my understanding is correct - it's just an "utilty" super class for three other views (so is Collection for Set, Mapping and Sequence), it's not a big deal IMHO. -- keywords: +patch Added file: https://bugs.python.org/file47354/mypatch.patch ___ Python tracker <https://bugs.python.org/issue32449> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32449] MappingView must inherit from Collection instead of Sized
New submission from Yahya Abou Imran : Quoting my own post on python-ideas: After I generate an UML diagram from collections.abc, I found very strange that MappingView inherit from Sized instead of Collection (new in python 3.6). Yes, MappingView only define __len__ and not __iter__ and __contains__, but all of its subclasses define them (KeysView, ValuesView and ItemViews). I tried to run the tests in test/test_collections.py after making this change and on only one fail : Traceback (most recent call last): File "/usr/lib/python3.6/test/test_collections.py", line 789, in test_Collection self.assertNotIsInstance(x, Collection) AssertionError: dict_values([]) is an instance of Wich is absolutely wrong, since in reality a dict_values instance has the behaviour of a Collection: >>> vals = {1:'a', 2: 'b'}.values() >>> 'a' in vals True >>> 'c' in vals False >>> len(vals) 2 >>> for val in vals: ... print(val) ... a b The only lack is that it doesn't define a __contains__ method: >>> '__contains__' in vals False It uses __iter__ to find the presence of the value. But, hey: we have register() for this cases! In fact, when MappingView inherit from Collection, dict_values is considered as a subclass of Collection since it's in the register of ValuesView, causing the above bug... So, the test have to be changed, and dict_values must be placed in the samples that pass the test, and not in the ones that fail it. -- messages: 309200 nosy: yahya-abou-im...@protonmail.com priority: normal severity: normal status: open title: MappingView must inherit from Collection instead of Sized type: enhancement versions: Python 3.6, Python 3.7 ___ Python tracker <https://bugs.python.org/issue32449> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com