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

Improve type hint annotations for @particle_input #2443

Merged
merged 15 commits into from Jan 10, 2024

Conversation

namurphy
Copy link
Member

@namurphy namurphy commented Jan 4, 2024

Description

This PR adds & improves type hint annotations in plasmapy.particles.decorators which is where @particle_input is defined, along with the tests.

There are some # type: ignore[...] statements for errors due to mypy's limitation as a static (rather than dynamic) type checker. We'll probably need to add that to the contributor guide.

Motivation and context

See #2434.

Related issues

This PR is a spinoff of #2429, and the motivation for #2442. Closes #2434.

@github-actions github-actions bot added the plasmapy.particles Related to the plasmapy.particles subpackage label Jan 4, 2024
Copy link

codecov bot commented Jan 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (63c8124) 96.92% compared to head (2037116) 96.93%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2443   +/-   ##
=======================================
  Coverage   96.92%   96.93%           
=======================================
  Files         104      104           
  Lines        9153     9163   +10     
=======================================
+ Hits         8872     8882   +10     
  Misses        281      281           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -351,7 +367,7 @@ def verify_charge_categorization(self, particle) -> None:

if isinstance(uncharged, Iterable):
uncharged = any(uncharged)
lacks_charge_info = any(lacks_charge_info)
lacks_charge_info = any(lacks_charge_info) # type: ignore[arg-type]
Copy link
Member Author

Choose a reason for hiding this comment

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

For some reason, the built-in any and all annotations aren't matching the types that we're using here.

Copy link
Contributor

Choose a reason for hiding this comment

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

🤔

Comment on lines +533 to +534
def f(particle: ParticleLike) -> Union[Particle, CustomParticle, ParticleList]:
return particle # type: ignore[return-value]
Copy link
Member Author

Choose a reason for hiding this comment

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

The # type: ignore comment here reflects that static type checkers like mypy are unable to tell that there is an implicit object conversion happening here, which would require actually running the code in @particle_input instead of statically analyzing it. This is going to show up in a bunch of places throughout the code. In these cases, it's probably fine to explicitly ignore the mypy error since it is something that mypy is unable to check.

@namurphy namurphy marked this pull request as ready for review January 5, 2024 00:08
@namurphy namurphy requested a review from a team as a code owner January 5, 2024 00:08
@namurphy namurphy requested review from ejohnson-96 and pheuer and removed request for a team and ejohnson-96 January 5, 2024 00:08
@@ -240,10 +254,10 @@ def annotations(self) -> dict[str, Any]:
-------
`dict` of `str` to `object`
"""
return self._data.get("annotations")
return self._data.get("annotations") # type: ignore[return-value]
Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't want to bother with fixing this since there's a new way to get annotations in Python 3.10, and we're dropping Python 3.9 support soon anyway.

Copy link
Contributor

@ejohnson-96 ejohnson-96 left a comment

Choose a reason for hiding this comment

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

👍

Comment on lines -329 to -330
[mypy-plasmapy.particles.decorators]
disable_error_code = arg-type,assignment,call-arg,index,misc,no-any-return,no-untyped-call,no-untyped-def,operator,return-value,type-arg,union-attr,var-annotated
Copy link
Member Author

Choose a reason for hiding this comment

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

These are the mypy errors that existed in plasmapy.particles.decorators before this pull request. My process was essentially to:

  1. Remove one error code and find the errors that mypy was reporting
  2. Either fix those errors, or add a #type: ignore comment if the reported error wasn't actually a problem with the code
  3. GOTO 1

@namurphy
Copy link
Member Author

Thank you for the reviews, @ejohnson-96!

@namurphy namurphy merged commit 6918db6 into PlasmaPy:main Jan 10, 2024
16 checks passed
@namurphy namurphy deleted the particle-input-typing branch January 10, 2024 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plasmapy.particles Related to the plasmapy.particles subpackage static type checking testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Get @particle_input to pass mypy's strict mode
2 participants