Skip to content

Use Typevar defaults for TaskStatus and Matcher - #3019

Merged
jakkdl merged 7 commits into
python-trio:masterfrom
TeamSpen210:typevar-defaults
Jun 19, 2024
Merged

jakkdl merged 7 commits into
python-trio:masterfrom
TeamSpen210:typevar-defaults

Conversation

@TeamSpen210

Copy link
Copy Markdown
Contributor

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.

@TeamSpen210 TeamSpen210 added the typing Adding static types to trio's interface label Jun 16, 2024
@codecov

codecov Bot commented Jun 16, 2024 •

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.63%. Comparing base (451393a) to head (87451f5).

Additional details and impacted files
@@           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 with missing lines Coverage Δ
src/trio/_core/_run.py 99.38% <100.00%> (ø)
src/trio/testing/_raises_group.py 100.00% <100.00%> (ø)

@TeamSpen210 TeamSpen210 changed the title Use Typevar defaults for TaskStatus, RaisesGroup and Matcher Use Typevar defaults for TaskStatus and Matcher Jun 16, 2024
@TeamSpen210

Copy link
Copy Markdown
Contributor Author

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

A5rocks commented Jun 16, 2024 •

Copy link
Copy Markdown
Contributor

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

@A5rocks A5rocks left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code makes sense, though I'm not sure either RaisesGroup or (especially) _ExceptionInfo is used in type hints enough to make typevar defaults make sense. Definitely for TaskStatus though.

Comment thread src/trio/_util.py Outdated
Comment thread src/trio/testing/_raises_group.py Outdated
@A5rocks

A5rocks commented Jun 16, 2024 •

Copy link
Copy Markdown
Contributor

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

jakkdl commented Jun 17, 2024

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Contributor Author

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

jakkdl commented Jun 18, 2024

Copy link
Copy Markdown
Member

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.

@jakkdl
jakkdl merged commit 26cc6ee into python-trio:master Jun 19, 2024
@TeamSpen210
TeamSpen210 deleted the typevar-defaults branch June 21, 2024 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

typing Adding static types to trio's interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants