Add typing for _core/_asyncgens.py#2735
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #2735 +/- ##
=======================================
Coverage 98.90% 98.90%
=======================================
Files 113 113
Lines 16686 16691 +5
Branches 3025 3025
=======================================
+ Hits 16504 16509 +5
Misses 125 125
Partials 57 57
|
|
I do have one issue currently, mypy says |
|
Now we have these errors with those changes: trio/_core/_asyncgens.py:30: error: Incompatible types in assignment (expression has type "WeakSet[_T]", variable has type "MutableSet[AsyncGeneratorType[object, NoReturn]]") [assignment]
trio/_core/_asyncgens.py:37: error: Incompatible types in assignment (expression has type "Set[_T]", variable has type "Set[AsyncGeneratorType[object, NoReturn]]") [assignment]
trio/_core/_asyncgens.py:184: error: Incompatible types in assignment (expression has type "Set[<nothing>]", variable has type "MutableSet[AsyncGeneratorType[object, NoReturn]]") [assignment] |
A5rocks
left a comment
A5rocks
left a comment
There was a problem hiding this comment.
Small followups but looks good. I think we could probably get rid of the nasty type-check-only time type aliases in a couple months after tools get fixed.
|
|
||
| self.prev_hooks = sys.get_asyncgen_hooks() | ||
| sys.set_asyncgen_hooks(firstiter=firstiter, finalizer=finalizer) | ||
| sys.set_asyncgen_hooks(firstiter=firstiter, finalizer=finalizer) # type: ignore[arg-type] # Finalizer doesn't use AsyncGeneratorType |
There was a problem hiding this comment.
Now that I look at the comment history I see why we use AsyncGeneratorType (that was my question).
However, if this is indeed guaranteed to be correct (I still don't understand but I haven't really looked too closely at docs), then can someone raise an error on typeshed too?
There was a problem hiding this comment.
Well any library could manually do sys.get_asyncgen_hooks() then call it with whatever, but that’s rather esoteric and probably their fault if they pass something wrong. In the standard library, it’s only called deep in the async generator code, when they’re being destroyed.
We could check, but I think it might actually be better not to - the only type that might make sense to pass here would be something like Cython, which would probably duck-type well enough.
jakkdl
left a comment
jakkdl
left a comment
There was a problem hiding this comment.
Up the strictness in pyproject.toml for the file
Changed in cd19e84 |
jakkdl
left a comment
jakkdl
left a comment
There was a problem hiding this comment.
Nothing to add aside from what's already up for discussion
|
🚀 |
5 participants
This PR adds type annotations to
_core/_asyncgens.pyI would love feedback and any comments on how this could be improved to be more accurate.