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

no untyped tests #8926

Closed
wants to merge 4 commits into from
Closed

no untyped tests #8926

wants to merge 4 commits into from

Conversation

Illviljan
Copy link
Contributor

  • Closes #xxxx
  • Tests added
  • User visible changes (including notable bug fixes) are documented in whats-new.rst
  • New functions/methods are listed in api.rst

@max-sixty
Copy link
Collaborator

max-sixty commented Apr 10, 2024

I'm don't think disallow_untyped_defs within tests gives us much, and adds a tax of -> None everywhere, especially for folks who aren't used to this. Tests rarely return anything, so having typed functions isn't that helpful.

check_untyped_defs is the big win, because then tests are used to check types. So any work to move modules into that list would be v helpful.

(IIRC disallow_untyped_defs was the only way of having all types checked before check_untyped_defs became available. So that was my original approach to the problem, but is now rendered obsolete..)

@Illviljan
Copy link
Contributor Author

Illviljan commented Apr 11, 2024

Yeah, I thought we always forced at least -> None nowadays.
I thought it was a good way to spot tests that weren't tested and taught users to ponder about what the function inputs and outputs.

Considering all the tricky ignores related to DataTree I find using check_untyped_defs too magical and far from default behavior, and here's the debug PR to figure it out.

@Illviljan Illviljan closed this Apr 14, 2024
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.

2 participants