Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type RaisesGroup properly #3141

Merged
merged 15 commits into from
Nov 27, 2024
21 changes: 0 additions & 21 deletions docs/source/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import glob
import os
import sys
import types
from pathlib import Path
from typing import TYPE_CHECKING, cast

Expand Down Expand Up @@ -152,16 +151,6 @@ def autodoc_process_signature(
return_annotation: str,
) -> tuple[str, str]:
"""Modify found signatures to fix various issues."""
if name == "trio.testing._raises_group._ExceptionInfo.type":
# This has the type "type[E]", which gets resolved into the property itself.
# That means Sphinx can't resolve it. Fix the issue by overwriting with a fully-qualified
# name.
assert isinstance(obj, property), obj
assert isinstance(obj.fget, types.FunctionType), obj.fget
assert (
obj.fget.__annotations__["return"] == "type[MatchE]"
), obj.fget.__annotations__
obj.fget.__annotations__["return"] = "type[~trio.testing._raises_group.MatchE]"
if signature is not None:
signature = signature.replace("~_contextvars.Context", "~contextvars.Context")
if name == "trio.lowlevel.RunVar": # Typevar is not useful here.
Expand All @@ -170,16 +159,6 @@ def autodoc_process_signature(
# Strip the type from the union, make it look like = ...
signature = signature.replace(" | type[trio._core._local._NoValue]", "")
signature = signature.replace("<class 'trio._core._local._NoValue'>", "...")
if name in ("trio.testing.RaisesGroup", "trio.testing.Matcher") and (
"+E" in signature or "+MatchE" in signature
):
# This typevar being covariant isn't handled correctly in some cases, strip the +
# and insert the fully-qualified name.
signature = signature.replace("+E", "~trio.testing._raises_group.E")
signature = signature.replace(
"+MatchE",
"~trio.testing._raises_group.MatchE",
)
if "DTLS" in name:
signature = signature.replace("SSL.Context", "OpenSSL.SSL.Context")
# Don't specify PathLike[str] | PathLike[bytes], this is just for humans.
Expand Down
1 change: 1 addition & 0 deletions newsfragments/3141.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix `trio.testing.RaisesGroup`'s typing.
2 changes: 1 addition & 1 deletion src/trio/_tests/test_testing_raisesgroup.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def test_raises_group() -> None:
f'Invalid argument "{TypeError()!r}" must be exception type, Matcher, or RaisesGroup.',
),
):
RaisesGroup(TypeError())
RaisesGroup(TypeError()) # type: ignore
A5rocks marked this conversation as resolved.
Show resolved Hide resolved

with RaisesGroup(ValueError):
raise ExceptionGroup("foo", (ValueError(),))
Expand Down
2 changes: 1 addition & 1 deletion src/trio/_tests/type_tests/path.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def sync_attrs(path: trio.Path) -> None:
assert_type(path.match("*.py"), bool)
assert_type(path.relative_to("/usr"), trio.Path)
if sys.version_info >= (3, 12):
assert_type(path.relative_to("/", walk_up=True), bool)
assert_type(path.relative_to("/", walk_up=True), trio.Path)
assert_type(path.with_name("filename.txt"), trio.Path)
assert_type(path.with_stem("readme"), trio.Path)
assert_type(path.with_suffix(".log"), trio.Path)
Expand Down
85 changes: 25 additions & 60 deletions src/trio/_tests/type_tests/raisesgroup.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,3 @@
"""The typing of RaisesGroup involves a lot of deception and lies, since AFAIK what we
actually want to achieve is ~impossible. This is because we specify what we expect with
instances of RaisesGroup and exception classes, but excinfo.value will be instances of
[Base]ExceptionGroup and instances of exceptions. So we need to "translate" from
RaisesGroup to ExceptionGroup.

The way it currently works is that RaisesGroup[E] corresponds to
ExceptionInfo[BaseExceptionGroup[E]], so the top-level group will be correct. But
RaisesGroup[RaisesGroup[ValueError]] will become
ExceptionInfo[BaseExceptionGroup[RaisesGroup[ValueError]]]. To get around that we specify
RaisesGroup as a subclass of BaseExceptionGroup during type checking - which should mean
that most static type checking for end users should be mostly correct.
"""

from __future__ import annotations

import sys
Expand All @@ -23,19 +9,6 @@
if sys.version_info < (3, 11):
from exceptiongroup import BaseExceptionGroup, ExceptionGroup

# split into functions to isolate the different scopes


def check_inheritance_and_assignments() -> None:
# Check inheritance
_: BaseExceptionGroup[ValueError] = RaisesGroup(ValueError)
_ = RaisesGroup(RaisesGroup(ValueError)) # type: ignore

a: BaseExceptionGroup[BaseExceptionGroup[ValueError]]
a = RaisesGroup(RaisesGroup(ValueError))
a = BaseExceptionGroup("", (BaseExceptionGroup("", (ValueError(),)),))
assert a

A5rocks marked this conversation as resolved.
Show resolved Hide resolved

def check_matcher_typevar_default(e: Matcher) -> object:
assert e.exception_type is not None
Expand All @@ -46,29 +19,34 @@ def check_matcher_typevar_default(e: Matcher) -> object:


def check_basic_contextmanager() -> None:
# One level of Group is correctly translated - except it's a BaseExceptionGroup
# instead of an ExceptionGroup.

with RaisesGroup(ValueError) as e:
raise ExceptionGroup("foo", (ValueError(),))
assert_type(e.value, BaseExceptionGroup[ValueError])
assert_type(e.value, ExceptionGroup[ValueError])


def check_basic_matches() -> None:
# check that matches gets rid of the naked ValueError in the union
exc: ExceptionGroup[ValueError] | ValueError = ExceptionGroup("", (ValueError(),))
if RaisesGroup(ValueError).matches(exc):
assert_type(exc, BaseExceptionGroup[ValueError])
assert_type(exc, ExceptionGroup[ValueError])

# also check that BaseExceptionGroup shows up for BaseExceptions
if RaisesGroup(KeyboardInterrupt).matches(exc):
assert_type(exc, BaseExceptionGroup[KeyboardInterrupt])


def check_matches_with_different_exception_type() -> None:
# This should probably raise some type error somewhere, since
# ValueError != KeyboardInterrupt
e: BaseExceptionGroup[KeyboardInterrupt] = BaseExceptionGroup(
"",
(KeyboardInterrupt(),),
)

# note: it might be tempting to have this warn.
# however, note that we should not warn if e: BaseException
# .... therefore this needs to pass as there is no distinction
if RaisesGroup(ValueError).matches(e):
assert_type(e, BaseExceptionGroup[ValueError])
A5rocks marked this conversation as resolved.
Show resolved Hide resolved
assert_type(e, ExceptionGroup[ValueError])


def check_matcher_init() -> None:
Expand Down Expand Up @@ -134,59 +112,46 @@ def handle_value(e: BaseExceptionGroup[ValueError]) -> bool:


def raisesgroup_narrow_baseexceptiongroup() -> None:
"""Check type narrowing specifically for the container exceptiongroup.
This is not currently working, and after playing around with it for a bit
I think the only way is to introduce a subclass `NonBaseRaisesGroup`, and overload
`__new__` in Raisesgroup to return the subclass when exceptions are non-base.
(or make current class BaseRaisesGroup and introduce RaisesGroup for non-base)
I encountered problems trying to type this though, see
https://github.com/python/mypy/issues/17251
That is probably possible to work around by entirely using `__new__` instead of
`__init__`, but........ ugh.
"""
"""Check type narrowing specifically for the container exceptiongroup."""

def handle_group(e: ExceptionGroup[Exception]) -> bool:
return True

def handle_group_value(e: ExceptionGroup[ValueError]) -> bool:
return True

# should work, but BaseExceptionGroup does not get narrowed to ExceptionGroup
RaisesGroup(ValueError, check=handle_group_value) # type: ignore
RaisesGroup(ValueError, check=handle_group_value)

# should work, but BaseExceptionGroup does not get narrowed to ExceptionGroup
RaisesGroup(Exception, check=handle_group) # type: ignore
RaisesGroup(Exception, check=handle_group)


def check_matcher_transparent() -> None:
with RaisesGroup(Matcher(ValueError)) as e:
...
_: BaseExceptionGroup[ValueError] = e.value
assert_type(e.value, BaseExceptionGroup[ValueError])
assert_type(e.value, ExceptionGroup[ValueError])


def check_nested_raisesgroups_contextmanager() -> None:
with RaisesGroup(RaisesGroup(ValueError)) as excinfo:
raise ExceptionGroup("foo", (ValueError(),))

# thanks to inheritance this assignment works
_: BaseExceptionGroup[BaseExceptionGroup[ValueError]] = excinfo.value
# and it can mostly be treated like an exceptiongroup
print(excinfo.value.exceptions[0].exceptions[0])

# but assert_type reveals the lies
A5rocks marked this conversation as resolved.
Show resolved Hide resolved
print(type(excinfo.value)) # would print "ExceptionGroup"
# typing says it's a BaseExceptionGroup
A5rocks marked this conversation as resolved.
Show resolved Hide resolved
assert_type(
excinfo.value,
BaseExceptionGroup[RaisesGroup[ValueError]],
ExceptionGroup[ExceptionGroup[ValueError]],
)

print(type(excinfo.value.exceptions[0])) # would print "ExceptionGroup"
# but type checkers are utterly confused
A5rocks marked this conversation as resolved.
Show resolved Hide resolved
assert_type(
excinfo.value.exceptions[0],
Union[RaisesGroup[ValueError], BaseExceptionGroup[RaisesGroup[ValueError]]],
Union[
ExceptionGroup[ValueError],
ExceptionGroup[ExceptionGroup[ValueError]],
],
)
A5rocks marked this conversation as resolved.
Show resolved Hide resolved


Expand All @@ -196,17 +161,17 @@ def check_nested_raisesgroups_matches() -> None:
"",
(ExceptionGroup("", (ValueError(),)),),
)
# has the same problems as check_nested_raisesgroups_contextmanager

if RaisesGroup(RaisesGroup(ValueError)).matches(exc):
assert_type(exc, BaseExceptionGroup[RaisesGroup[ValueError]])
assert_type(exc, ExceptionGroup[ExceptionGroup[ValueError]])


def check_multiple_exceptions_1() -> None:
a = RaisesGroup(ValueError, ValueError)
b = RaisesGroup(Matcher(ValueError), Matcher(ValueError))
c = RaisesGroup(ValueError, Matcher(ValueError))

d: BaseExceptionGroup[ValueError]
d: RaisesGroup[ValueError]
d = a
d = b
d = c
Expand All @@ -219,7 +184,7 @@ def check_multiple_exceptions_2() -> None:
b = RaisesGroup(Matcher(ValueError), TypeError)
c = RaisesGroup(ValueError, TypeError)

d: BaseExceptionGroup[Exception]
d: RaisesGroup[Exception]
d = a
d = b
d = c
Expand Down
Loading
Loading