[issue35456] asyncio.Task.set_result() and set_exception() missing docstrings (and Liskov sub. principle)

[Yury Selivanov]

Yury Selivanov  added the comment:

-1 on this; there is no clear win in doing this refactoring, only a hard to 
estimate chance of making a regression.

Yahya, feel free to tackle other asyncio bugs or improvements, this one is just 
something that we aren't comfortable doing right now.

--
resolution:  -> postponed
stage:  -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35456] asyncio.Task.set_result() and set_exception() missing docstrings (and Liskov sub. principle)

[INADA Naoki]

INADA Naoki  added the comment:

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

I'm not against it, unless it doesn't have backward incompatibility
or performance regression.

But I'm not sure you estimate the difficulty correctly: there are C 
implementation of Future and Task.  You need to have deep knowledge of Python/C 
APIs.


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

Personally speaking, I dislike treating OOP principles like Ten Commandments.  
Principles have some reasons.  And these reasons are reasonable not for all 
cases.  When people say "it's bad because it violates principle!", they may 
have poor knowledge about the prinicple.
If they really know the principle, they must describe real-world problem caused 
by the violation.

In this case, I agree that misleading docstring is a small real-world problem 
caused by the violation.  While it can be fixable without fixing the violation.

Generally, `set_result` or `set_exception` is called by the creator of the 
Future.  So requiring knowledge of concrete class is not a big problem.
On the other hand, awaiting future object without knowing concrete class is 
common.  But Task is awaitable.  So there are no problem here.

--
nosy: +inada.naoki

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35456] asyncio.Task.set_result() and set_exception() missing docstrings (and Liskov sub. principle)

[Ned Deily]

Change by Ned Deily :


--
components: +asyncio
nosy: +asvetlov, yselivanov

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35456] asyncio.Task.set_result() and set_exception() missing docstrings (and Liskov sub. principle)

[Yahya Abou Imran]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com