Skip to content

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

Merged
agronholm merged 8 commits intoagronholm:mainfrom
jakkdl:incorrect_self_import
Jan 10, 2024
Merged

expand CI pyright to use full python version matrix and fix type errors#101
agronholm merged 8 commits intoagronholm:mainfrom
jakkdl:incorrect_self_import

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 force-pushed the incorrect_self_import branch from e7b049d to f99aa0e Compare December 23, 2023 12:29
@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.

@jakkdl jakkdl requested a review from agronholm January 10, 2024 15:32
@agronholm agronholm merged commit 0604a05 into agronholm:main Jan 10, 2024
@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