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.

Sign up for GitHub

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

Jump to bottom

Add runtime type checking in CI tests #9745

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Add runtime type checking in CI tests #9745

wants to merge 2 commits into from
+45 −2

Conversation

DimitriPapadopoulos
Copy link
Contributor

This should prevent TYPE_CHECKING misuse.

Fixes #9744.

@DimitriPapadopoulos DimitriPapadopoulos force-pushed the pytest-beartype branch 2 times, most recently from 7c7eee9 to 4b2f732 Compare November 7, 2024 17:33
keewis
keewis reviewed Nov 7, 2024
View reviewed changes
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
@max-sixty
Copy link
Collaborator

This is a good test! Unfortunately our CI fails, and we couldn't work out a succinct way of changing our annotations so that we could resolve #9581.

If there were some way of adding this in part, that would be great. But likely we'll need to wait for more support so we can support this while having just DataArray in the type definition without the full path (more discussion in the attached issue)

@headtr1ck
Copy link
Collaborator

Maybe we make it optional, similar to pyright?

@DimitriPapadopoulos
Copy link
Contributor Author

I was planning on exploring the suggestions in #9581.

But yes, I could make the test optional for now.

@DimitriPapadopoulos DimitriPapadopoulos force-pushed the pytest-beartype branch 2 times, most recently from 21779df to 1c7d925 Compare November 10, 2024 12:56
@DimitriPapadopoulos
Add runtime type checking in CI tests
2aaf037 
This should prevent TYPE_CHECKING misuse.

It might also found issues not found by static type checking,
typically by the current MyPy.

Make it optional for now, while we fix issues.
@max-sixty
Copy link
Collaborator

Is it helpful to have an optional CI which fails everytime at the first import though?

@DimitriPapadopoulos
Copy link
Contributor Author

It looks like solutions that might work with beartype fail with MyPy. I haven't been able to find a solution within the little time I can dedicate to this issue.

In any case, it looks like TYPE_CHECKING shouldn't be used, except to avoid circular imports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keewis keewis keewis left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add runtime type checking
4 participants
@DimitriPapadopoulos @max-sixty @headtr1ck @keewis