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

expand CI pyright to use full python version matrix and fix type errors #101

Merged
merged 8 commits into from
Jan 10, 2024

Conversation

jakkdl
Copy link
Contributor

@jakkdl jakkdl commented Dec 23, 2023

(oops, meant to create this in my fork at first - sorry for the messiness in the PR)

It's perhaps a bit overkill to run pyright on the full test matrix, but with one backport from 3.11 and one from 3.12 we at least need to run <3.11, 3.11 and 3.12

@coveralls
Copy link

coveralls commented Dec 23, 2023

Pull Request Test Coverage Report for Build 7477159334

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.2%) to 98.373%

Totals Coverage Status
Change from base Build 7460984080: -0.2%
Covered Lines: 544
Relevant Lines: 553

💛 - Coveralls

@jakkdl jakkdl closed this Dec 23, 2023
@jakkdl jakkdl reopened this Dec 23, 2023
@jakkdl jakkdl changed the title temporarily expand CI pyright to full python version matrix to check output expand CI pyright to full python version Dec 23, 2023
@jakkdl jakkdl changed the title expand CI pyright to full python version expand CI pyright to use python version matrix Dec 23, 2023
@jakkdl jakkdl changed the title expand CI pyright to use python version matrix expand CI pyright to use full python version matrix and fix type errors Dec 23, 2023
@jakkdl
Copy link
Contributor Author

jakkdl commented Dec 23, 2023

Only warnings raised by pyright now is about missing docstrings, but --verifytypes doesn't return a failing exit code for that.

@jakkdl
Copy link
Contributor Author

jakkdl commented Dec 23, 2023

Not sure why coveralls thought this PR reduced coverage by -0.2%, but that made me take a look (after figuring out how to run it locally, since the website ain't displaying the source for me) which made me aware of "@overload" not being excluded. So fixed that as well.

Comment on lines 13 to 14
_ExceptionGroupSelf = TypeVar("_ExceptionGroupSelf", bound="ExceptionGroup")
_BaseExceptionGroupSelf = TypeVar("_BaseExceptionGroupSelf", bound="ExceptionGroup")
Copy link
Owner

Choose a reason for hiding this comment

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

Does this really require two different typevars? Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, set the wrong bound on the second one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But yeah we need two different TypeVars, the signature of e.g. ExceptionGroup.__new__ and BaseExceptionGroup.__new__ should be different - the former will return a [subclass of] ExceptionGroup, the latter a [subclass of] BaseExceptionGroup.

@jakkdl
Copy link
Contributor Author

jakkdl commented Jan 10, 2024

bump? :)

@agronholm
Copy link
Owner

I'll try to get to this today. I have so many projects to look after.

Copy link
Owner

@agronholm agronholm left a comment

Choose a reason for hiding this comment

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

Looks mostly fine, just a few changes needed.

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
src/exceptiongroup/_suppress.py Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@jakkdl jakkdl requested a review from agronholm January 10, 2024 15:32
@agronholm agronholm merged commit 0604a05 into agronholm:main Jan 10, 2024
15 checks passed
@agronholm
Copy link
Owner

Thanks!

@jakkdl jakkdl deleted the incorrect_self_import branch January 11, 2024 15:19
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.

Incorrect Self import breaks pyright type-checking
3 participants