Type completeness improvement (timeouts, CancelScope, and more)#2671
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #2671 +/- ##
=======================================
Coverage 98.84% 98.84%
=======================================
Files 113 113
Lines 16477 16484 +7
Branches 3003 3003
=======================================
+ Hits 16286 16293 +7
Misses 134 134
Partials 57 57
|
|
Didn't see anything of interest running @A5rocks what is the ideal size for pure typing PRs like this, in your opinion? I could easily continue and chug through tons of symbols, but don't want to saddle you with huge code reviews. |
| "trio._sync.Event.is_set", | ||
| "trio._sync.Event.wait", | ||
| "trio._sync.Event.statistics", | ||
| "trio._sync.CapacityLimiter", |
There was a problem hiding this comment.
Most/all of these random classes being type complete is due to Final.__new__ and NoPublicConstructor.__call__ getting typed.
|
I don't really have a preferred size given I'm not too used to typing PRs. (Basically I don't really think I care about length, just about having the context to check types!) |
|
huh, what's going on with codecov? Maybe something about run getting canceled? |
A5rocks
left a comment
A5rocks
left a comment
There was a problem hiding this comment.
I think most of the changes aren't objectionable, but a couple small comments.
| @@ -1,4 +1,5 @@ | |||
| # Little utilities we use internally | |||
| from __future__ import annotations | |||
There was a problem hiding this comment.
Should we preemptively stick this at the top of all files so that these kinds of change is already done?
There was a problem hiding this comment.
Actually IDK how I feel about using the cooler syntax early: we could use typing.List etc. and already-mentioned pyupgrade will drop that for newer syntax the moment we drop 3.8.
On the other hand it's more readable and more understandable. Though I guess List[int] vs list[int] (using from typing import ...) isn't too big a difference.
There was a problem hiding this comment.
The biggest downside to using typing.List is the additional overhead in import management - which is fully manual given our current lack of isort. This can be quite frustrating IME.
idc much whether to preemptively adding it
There was a problem hiding this comment.
I've liked using https://pypi.org/project/flake8-type-checking/ (or https://pypi.org/project/ruff/ ) for managing this.
There was a problem hiding this comment.
fucking github. anyway there's never a reason not to use from __future__ import annotations so you might as well just use 3.11+ typing syntax always.
There was a problem hiding this comment.
python-trio/flake8-async#78 (review)
Instagram/LibCST#870
There's some scenarios where it matters, but I think we're fine.
It looks like codecov broke something: https://github.com/python-trio/trio/actions/runs/5378612122/jobs/9758650074?pr=2671#step:6:39 I've just rerun the jobs in case that fixes it. Edit: I've investigated and found: codecov/feedback#126. |
jakkdl
force-pushed
the
typing_improvements
branch
from
421f46f to
2fd06b9
Compare
jakkdl
force-pushed
the
typing_improvements
branch
from
56238aa to
5195433
Compare
|
That's my comments addressed fine. |
5 participants
Just going down the list of untyped symbols, don't think there's much anything tricky. Will run trio-typing/stubtest against the changes