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

Fix typings of ExceptionGroup and BaseExceptionGroup #9230

Merged
merged 13 commits into from
Nov 23, 2022

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Nov 20, 2022

There are a lot of things going on in this PR:

  1. Methods no longer return Self type, because of how C implementation works
  2. split and subgroup require Self argument to be in callable, because of how C implementation works
  3. Some casts from BaseExceptionGroup to and from ExceptionGroup are supported. But not all of them

This is a very complex PR. I would love to provide any of my help to the reviewers.

@sobolevn
Copy link
Member Author

Ok, pyright has only 10 problems:

/home/runner/work/typeshed/typeshed/test_cases/stdlib/builtins/check_exception_group.py
  /home/runner/work/typeshed/typeshed/test_cases/stdlib/builtins/check_exception_group.py:28:13 - error: "assert_type" mismatch: expected "BaseExceptionGroup[BaseException] | None" but received "BaseExceptionGroup[KeyboardInterrupt | SystemExit] | None" (reportGeneralTypeIssues)
  /home/runner/work/typeshed/typeshed/test_cases/stdlib/builtins/check_exception_group.py:57:13 - error: "assert_type" mismatch: expected "ExceptionGroup[Exception] | None" but received "ExceptionGroup[ValueError | KeyError] | None" (reportGeneralTypeIssues)
  /home/runner/work/typeshed/typeshed/test_cases/stdlib/builtins/check_exception_group.py:88:5 - error: "assert_type" mismatch: expected "tuple[BaseExceptionGroup[BaseException] | None, BaseExceptionGroup[SystemExit] | None]" but received "tuple[BaseExceptionGroup[KeyboardInterrupt | SystemExit] | None, BaseExceptionGroup[SystemExit] | None]" (reportGeneralTypeIssues)
  /home/runner/work/typeshed/typeshed/test_cases/stdlib/builtins/check_exception_group.py:173:13 - error: "assert_type" mismatch: expected "ExceptionGroup[Exception] | None" but received "ExceptionGroup[ValueError | KeyError] | None" (reportGeneralTypeIssues)
  /home/runner/work/typeshed/typeshed/test_cases/stdlib/builtins/check_exception_group.py:198:5 - error: "assert_type" mismatch: expected "tuple[ExceptionGroup[Exception] | None, ExceptionGroup[ValueError | KeyError] | None]" but received "tuple[ExceptionGroup[TypeError | IndexError] | None, ExceptionGroup[ValueError | KeyError] | None]" (reportGeneralTypeIssues)
  /home/runner/work/typeshed/typeshed/test_cases/stdlib/builtins/check_exception_group.py:252:13 - error: "assert_type" mismatch: expected "BaseExceptionGroup[BaseException] | None" but received "BaseExceptionGroup[KeyboardInterrupt | SystemExit] | None" (reportGeneralTypeIssues)
  /home/runner/work/typeshed/typeshed/test_cases/stdlib/builtins/check_exception_group.py:255:13 - error: "assert_type" mismatch: expected "ExceptionGroup[Exception] | None" but received "ExceptionGroup[KeyError | TypeError] | None" (reportGeneralTypeIssues)
  /home/runner/work/typeshed/typeshed/test_cases/stdlib/builtins/check_exception_group.py:279:13 - error: "assert_type" mismatch: expected "tuple[ExceptionGroup[Exception] | None, BaseExceptionGroup[ValueError] | None]" but received "tuple[ExceptionGroup[TypeError | IndexError] | None, BaseExceptionGroup[ValueError] | None]" (reportGeneralTypeIssues)
  /home/runner/work/typeshed/typeshed/test_cases/stdlib/builtins/check_exception_group.py:328:13 - error: "assert_type" mismatch: expected "ExceptionGroup[Exception] | None" but received "ExceptionGroup[KeyError | TypeError] | None" (reportGeneralTypeIssues)
  /home/runner/work/typeshed/typeshed/test_cases/stdlib/builtins/check_exception_group.py:347:13 - error: "assert_type" mismatch: expected "tuple[ExceptionGroup[Exception] | None, ExceptionGroup[ValueError] | None]" but received "tuple[ExceptionGroup[TypeError | IndexError] | None, ExceptionGroup[ValueError] | None]" (reportGeneralTypeIssues)
10 errors, 0 warnings, 0 informations 

I will add a version guard later on.

@github-actions

This comment has been minimized.

@sobolevn
Copy link
Member Author

Looks like we need some other solution for unused # type: ignore comments.

 Running mypy --platform darwin --python-version 3.11 on the test cases for 'invoke'... success
Running mypy --platform darwin --python-version 3.11 on the test cases for 'requests'... success
Running mypy --platform darwin --python-version 3.11 on the standard library test cases... success
Running mypy --platform darwin --python-version 3.10 on the test cases for 'invoke'... success
Running mypy --platform darwin --python-version 3.10 on the test cases for 'requests'... success
Running mypy --platform darwin --python-version 3.10 on the standard library test cases... failure

test_cases/stdlib/builtins/check_exception_group.py:47: error: Unused "type: ignore" comment
test_cases/stdlib/builtins/check_exception_group.py:48: error: Unused "type: ignore" comment
test_cases/stdlib/builtins/check_exception_group.py:67: error: Unused "type: ignore" comment
test_cases/stdlib/builtins/check_exception_group.py:71: error: Unused "type: ignore" comment
...

@github-actions

This comment has been minimized.

test_cases/stdlib/builtins/check_exception_group.py Outdated Show resolved Hide resolved

# `BaseExceptionGroup` can work with `Exception`:
beg2 = BaseExceptionGroup("x", [ValueError()])
# FIXME: this is not right, runtime returns `ExceptionGroup` instance instead,
Copy link
Member

Choose a reason for hiding this comment

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

Should be doable by overriding __new__ (python/mypy#7188), did you try that?

Copy link
Member Author

@sobolevn sobolevn Nov 21, 2022

Choose a reason for hiding this comment

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

We have three cases:

  • BaseExceptionGroup.__new__ can return BaseExceptionGroup for _BaseExceptionT_co
  • BaseExceptionGroup.__new__ can return ExceptionGroup for _ExceptionT_co
  • BaseExceptionGroup.__new__ can return custom subclass

Either the first two work or the third one. Suggestions are welcome

test_cases/stdlib/builtins/check_exception_group.py Outdated Show resolved Hide resolved
test_cases/stdlib/builtins/check_exception_group.py Outdated Show resolved Hide resolved
test_cases/stdlib/builtins/check_exception_group.py Outdated Show resolved Hide resolved
test_cases/stdlib/builtins/check_exception_group.py Outdated Show resolved Hide resolved
test_cases/stdlib/builtins/check_exception_group.py Outdated Show resolved Hide resolved
@AlexWaygood
Copy link
Member

AlexWaygood commented Nov 21, 2022

I have an idea for how to fix the test cases on mypy; I'll get to work on a fix.

For flake8: would an alternative fix be to have from builtins import BaseExceptionGroup, ExceptionGroup at the top of the file?

@github-actions

This comment has been minimized.

@sobolevn
Copy link
Member Author

@AlexWaygood, thank you, flake8 idea worked!

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@AlexWaygood
Copy link
Member

Hooray, CI is finally green 🎉

@JelleZijlstra JelleZijlstra merged commit ae58142 into python:main Nov 23, 2022
@sobolevn
Copy link
Member Author

I really hope this is correct now! Ping me in case of any problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants