Repository navigation
Clean up type hints for subprocess - #3557
Conversation
d5c8a79 to
18280f4
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3557 +/- ##
===============================================
Coverage 100.00000% 100.00000%
===============================================
Files 128 128
Lines 19480 19480
Branches 1323 1323
===============================================
Hits 19480 19480
🚀 New features to boost your workflow:
|
| trio_stdin: ClosableSendStream | None = None | ||
| trio_stdout: ClosableReceiveStream | None = None | ||
| trio_stderr: ClosableReceiveStream | None = None | ||
| trio_stdin = None |
There was a problem hiding this comment.
I am surprised mypy strict mode is alright with this (then again I don't think we have that turned on be default)
Seeing this here and knowing this is likely a mypy strict mode thing, I wouldn't be surprised if this is one of my contributions for a future where we do enable strict mode
There was a problem hiding this comment.
mypy has a ton of special behavior around None where it will look around at context to figure out the right type hint.
See https://mypy-play.net/?mypy=latest&python=3.12&flags=strict&gist=2c203611ecce92bee7a12ef01ab7d524 for an example. (Also at some point I might switch us to using allow-redefinition, mypy's done a new version of it since I last tried that and probably it's less buggy!)
There was a problem hiding this comment.
Gotcha. Looks like things have changed from what I remember we had to do previously.
Zac-HD
left a comment
Zac-HD
left a comment
There was a problem hiding this comment.
lgtm, though it'd be nice to include a link to the pyright issue
18280f4 to
b851385
Compare
Followup for my review of #3555