Make asyncio.Task covariant

[Dreamsorcerer]

python/mypy#13689 (comment)

[hauntsaninja]

See my comment on the mypy issue, I think making Future covariant is unsound

[Dreamsorcerer]

Will it still work by making Task covariant and Future not?

[JelleZijlstra]

Thanks @hauntsaninja, I missed that method. We can't make Task covariant either, because Task.set_result would still be unsafe.

[hauntsaninja]

Task.set_result always raises, so I think you could get away with that.

[Dreamsorcerer]

Indeed, docs says:

Task inherits from Future all of its APIs except Future.set_result() and Future.set_exception().

[JelleZijlstra]

I'm still hesitant about having a covariant subclass of an invariant base class. Given B <: A, we'd have Task[A] <: Future[A], Task[B] <: Future[B], and Task[B] <: Task[A], but not Future[B] <: Future[A]. That feels unsound.

@erictraut curious if you have any opinions here: maybe type checkers should not allow covariant subclasses of invariant base classes? And how would this work under PEP 695?

[hauntsaninja]

It definitely feels sketchy, but in this particular case as far as I can tell it's not any more unsound than the existing Liskov issue with set_result

[erictraut]

I agree this is sketchy. Neither mypy nor pyright (I haven't tried the other type checkers) complain about this situation currently, even though it's clearly wrong.

fromtypingimportTypeVarT=TypeVar("T", covariant=True)
classMyList(list[T]): passdefappend_float(y: MyList[float]):
y.append(1.0)
x: MyList[int] =MyList([1, 2, 3])
append_float(x)

This is a pretty big hole in type safety, and it should probably be plugged. I'll explore a solution in pyright and see whether it produces a lot of noise for internal Microsoft code bases that use pyright.

In the meantime, I recommend not taking a dependency on this sketchy behavior — especially for classes that are as commonly-used as Task and Future.

[erictraut]

I've implemented support for this missing check in pyright. I didn't see any violations of it in our internal Microsoft source bases, nor were there any existing violations in typeshed.

I'm not sure what I'd recommend for this PR. I understand the reasoning, but I think I'd reject it because it's unsound from a type perspective, and it will no longer pass type checking when using pyright. If you decide to keep it, you can use a # type: ignore or # pyright: ignore comment to suppress the error.

[hauntsaninja]

Thanks for checking that!

I'm still in favour of this PR because it solves a real problem, models the runtime better, and there isn't opportunity for unsoundness beyond the pre-existing Liskov violation. Seems worth adding a test case to confirm behaviour, though.

But it is icky and we've managed so far without it, so if we were to reject that would be understandable.

Edit: I added the check to mypy in python/mypy#13714

[Akuli]

I don't have a strong opinion on this, but I agree that this can be useful. In one project, I had many collections of tasks that I wanted to cancel when a specific thing happened, so I made it something like list[asyncio.Task[Any]]. But because I never wanted to access the results of any of these tasks, and in fact I would want an error if I assume the results are of any specific type, asyncio.Task[object] would have been better if it worked.

[AlexWaygood]

I agree that doing this is usually unsound, and I'm very happy that we're fixing mypy and pyright to properly detect this unsoundness in the future. But I also agree that this case is weird enough that the usual rules don't really apply here, and that this models the runtime better. I'm happy for this to go in; we can type: ignore it when we upgrade to versions of mypy/pyright that detect this kind of unsoundness.

[github-actions]

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

[hauntsaninja]

This has been open for a bit and while it is a sketchy change, no one seems particularly opposed to it.

So I'm planning on merging. tldr:

• solves a real problem encountered by multiple people
• models the runtime better
• there isn't opportunity for unsoundness beyond the pre-existing Liskov violation
• two maintainers in favour, no maintainers clearly opposed