Closed vxgmichel closed 1 year ago
Following on from conversation in the issue, I had a play with a reasonably trivial test script (combine two generators, stream, print results). The current version seems to fall apart in mypy and pycharm slightly with regards to the generics.
$ mypy --strict typing-test.py
typing-test.py:20: error: Argument 1 to "Stream" has incompatible type "AsyncIterable[int]"; expected "AsyncIterable[T]" [arg-type]
typing-test.py:20: error: Argument 2 to "Stream" has incompatible type "AsyncIterable[int]"; expected "AsyncIterable[T]" [arg-type]
typing-test.py:24: error: "T" has no attribute "real" [attr-defined]
typing-test.py:24: error: "T" has no attribute "imag" [attr-defined]
typing-test.py:27: note: Revealed type is "def (*args: builtins.int, *, interval: builtins.float =) -> typing.AsyncIterator[builtins.int]"
typing-test.py:28: note: Revealed type is "def [Q] (*Q.args, **Q.kwargs) -> def (typing.AsyncIterable[builtins.int]) -> aiostream.core.Stream[[typing.AsyncIterable[T`2], *args: builtins.int, *, interval: builtins.float =], builtins.int]"
Found 4 errors in 1 file (checked 1 source file)
I had run into a similar problem on my own attempt, and solved it by replacing the return of a type[Stream[P, T]]
with a protocol that describes the known facts about the returned object -- namely a) if you call it with the parameters P
, you get a Stream[P, T]
(which is what an object creation boils down to), and it has a callable raw
that takes P
and returns AI[T]
.
Swapping to that model seems to have solves the typing issues -- though PyCharm is definitely still confused in some places. I believe it would also allow recombining Stream
and BaseStream
except for the dummy Stream.__init__
function that does not take a Factory
.
Proposed change: https://github.com/vxgmichel/aiostream/compare/typing...javajawa:aiostream:typing-with-protocol?expand=1
This results in the following mypy output (including the one expected type error):
$ mypy --strict typing-test.py
typing-test.py:27: note: Revealed type is "def (*args: builtins.int, *, interval: builtins.float =) -> typing.AsyncIterator[builtins.int]"
typing-test.py:28: error: "StreamOperator[[VarArg(int), DefaultNamedArg(float, 'interval')], int]" has no attribute "pipe" [attr-defined]
typing-test.py:28: note: Revealed type is "Any"
Found 1 error in 1 file (checked 1 source file)
A re-look and I have noticed that both IDE and MyPy is struggling with .pipe
resolution on my branch. Will take a look at some point.
queue = asyncio.Queue()
xs = stream.range(3) | add_resource.pipe(1) | pipe.action(queue.put)
tests/test_misc.py:22: error: Unsupported operand types for | ("Stream[int]" and "Stream[Any]") [operator]
tests/test_misc.py:22: error: Argument 1 has incompatible type "int"; expected "AsyncIterable[Any]" [arg-type]
Thanks a lot for the report, it's much appreciated :)
I'll look into it when I'll have a couple of hours to spare
I looked a bit into your commit and it looks like you're definitely on the right track with Protocol
:+1:
I wanted to make sure that it's currently possible to correctly type this operator/pipe operator logic so I started with a simpler example: https://gist.github.com/vxgmichel/765b0698fd6041f3c16379439b5bebeb
It looks like it works fine! In particular, I was impressed that the following definition works flawlessly:
class PipableOperator(Operator[Concatenate[Runable[X], P], T]):
I'll try to apply the same logic on top of your commit later next week and see if it fixes the problem you mentioned.
@javajawa I spent a bit more time on the typing issue and using protocols for typing the dynamically created types made a lot of sense, thanks :) This allowed me to remove P
from the BaseStream
generic parameters, which was a mistake in the first place.
Sadly I ran into a mypy bug that I reported here: https://github.com/python/mypy/issues/15753 I haven't found a workaround for it yet, but feel free to have a look at it, maybe I missed something :)
@javajawa It should be good now, could you give it a try? thanks :)
Fix #36