Add type annotations to most functions #35
Conversation
goodboy
force-pushed
the
type_annotations
branch
from
f1a8497
to
718cd21
Compare
This is purely for documentation purposes for now as it should be obvious a bunch of the signatures aren't using the correct "generics" syntax (i.e. the use of `(str, int)` instead of `typing.Tuple[str, int])`) in a bunch of places. We're also not using a type checker yet and besides, `trio` doesn't really expose a lot of its internal types very well. 2SQASH
goodboy
force-pushed
the
type_annotations
branch
from
af18432
to
c3eee1f
Compare
|
I wouldn't do it like this, imho. Stick to what mypy/typing module wants, even if its more verbose. Using But the fact that this design not only deviates away from community's approach to this problem, but is actively hostile to using I'd personally reject this in any project under my control, so not interested in reviewing it. |
|
@vodik changed it all. I'm just gonna go ahead and try to get |
|
ping @vodik. Should be running |
goodboy
force-pushed
the
type_annotations
branch
from
05abb9f
to
4d63125
Compare
| async with self._send_lock: | ||
| return await self.stream.send_all( | ||
| msgpack.dumps(data, use_bin_type=True)) | ||
|
|
||
| async def get(self): | ||
| async def get(self) -> Any: |
There was a problem hiding this comment.
Might want to look and see if you can define a TypeVar for this instead: https://mypy.readthedocs.io/en/latest/generics.html
T = TypeVar('T')
class StreamQueue(Generic[T]):
async def get(self) -> Any:
passYou can get a little bit more type safety this way if you know what the return type will be situationaly:
queue = StreamQueue[str](steam)For a StreamQueue that operates over strings.
There was a problem hiding this comment.
Hmm yeah I was thinking I might want to more strictly define the underlying IPC protocol being used. So maybe this can tie in with that. Do you think it's necessary to type the msgpack blobs as well?
| self._expect_result = None | ||
| self._exc: Optional[RemoteActorError] = None | ||
| self._expect_result: Optional[ | ||
| Tuple[str, Any, str, Dict[str, Any]] |
There was a problem hiding this comment.
You can do something like this:
Result = Tuple[str, Any, str, Dict[str, Any]
self._expect_result: Optional[Result]There was a problem hiding this comment.
You think this is necessary though?
| fn: typing.Callable, | ||
| bind_addr: Tuple[str, int] = ('127.0.0.1', 0), | ||
| rpc_module_paths: List[str] = None, | ||
| statespace: dict = None, |
| @@ -274,13 +287,13 @@ def do_hard_kill(proc): | |||
|
|
|||
|
|
|||
| @asynccontextmanager | |||
| async def open_nursery(supervisor=None): | |||
| async def open_nursery() -> typing.AsyncGenerator[None, ActorNursery]: | |||
There was a problem hiding this comment.
Got the arguments backwards? Isn't the second argument the send type? I see you yielding a nursery.
There was a problem hiding this comment.
yupp I remember forgetting about this now..
weird that mpy didn't catch it? I guess asynccontextmanager isn't exactly compatible yet.
This is purely for documentation purposes for now as it should be
obvious a bunch of the signatures aren't using the correct "generics"
syntax (i.e. the use of
(str, int)instead oftyping.Tuple[str, int]))in a bunch of places. We're also not using a type checker yet and besides,
triodoesn't really expose a lot of its internal types very well.cc @vodik