python-trio / trio

Trio – a friendly Python library for async concurrency and I/O
https://trio.readthedocs.io
Other
5.98k stars 325 forks source link

Use Typevar defaults for `TaskStatus` and `Matcher` #3019

Closed TeamSpen210 closed 1 week ago

TeamSpen210 commented 2 weeks ago

This causes annotations to default to TaskStatus[None], RaisesGroup[BaseException] and Matcher[BaseException], instead of Any. _socket could also use this to make SocketType generic over AddressFormat, but I'm not really sure how that should behave.

codecov[bot] commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.63%. Comparing base (451393a) to head (87451f5). Report is 2 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #3019 +/- ## ======================================= Coverage 99.63% 99.63% ======================================= Files 120 120 Lines 17863 17865 +2 Branches 3217 3216 -1 ======================================= + Hits 17798 17800 +2 Misses 46 46 Partials 19 19 ``` | [Files](https://app.codecov.io/gh/python-trio/trio/pull/3019?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio) | Coverage Δ | | |---|---|---| | [src/trio/\_core/\_run.py](https://app.codecov.io/gh/python-trio/trio/pull/3019?src=pr&el=tree&filepath=src%2Ftrio%2F_core%2F_run.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio#diff-c3JjL3RyaW8vX2NvcmUvX3J1bi5weQ==) | `99.38% <100.00%> (ø)` | | | [src/trio/testing/\_raises\_group.py](https://app.codecov.io/gh/python-trio/trio/pull/3019?src=pr&el=tree&filepath=src%2Ftrio%2Ftesting%2F_raises_group.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio#diff-c3JjL3RyaW8vdGVzdGluZy9fcmFpc2VzX2dyb3VwLnB5) | `100.00% <100.00%> (ø)` | |
TeamSpen210 commented 2 weeks ago

Seems RaisesGroup needs to be Any right now, otherwise a bunch of type-tests break for Pyright. @jakkdl looks like it might not actually be inferring the generic, and previously I think it wasn't generic at all.

A5rocks commented 2 weeks ago

RaisesGroup types are currently all sorts of messed up IMO, though we still have to wait on https://github.com/python/mypy/pull/16753 + a release of mypy before making them better...

A5rocks commented 2 weeks ago

Turns out the stuff necessary for RaisesGroup-but-good is merged in mypy (in someone else's PR that touched inference)! After the next mypy release I'll take my chances on nicer typing. I don't think it's worth thinking too much about type var defaults for them right now.

jakkdl commented 2 weeks ago

Seems RaisesGroup needs to be Any right now, otherwise a bunch of type-tests break for Pyright. @jakkdl looks like it might not actually be inferring the generic, and previously I think it wasn't generic at all.

could you maybe add a test (with type: ignore or pyright: ignore) that demonstrates the issue, even without default?

TeamSpen210 commented 2 weeks ago

Looking at the error again, it might just be the mypy issue we're already aware of. I'll just revert the changes to RaiseGroup for this PR.

jakkdl commented 2 weeks ago

It would be nice to have some addition to the typing tests that demonstrate what has changed.

def check_typevar_default(e: Matcher) -> None:
    assert e.exception_type is not None
    f = e.exception_type()
    # this would previously pass, as `f` would be `Any`
    f = 5  # type: ignore[assignment]
def check_typevar_default_explicit(e: Matcher) -> None:
    assert_type(e.exception_type, Optional[type[BaseException]])

Other than that it looks great.