Skip to content

Improve type hint annotations for @particle_input - #2443

Merged
namurphy merged 15 commits into
PlasmaPy:mainfrom
namurphy:particle-input-typing
Jan 10, 2024
Merged

namurphy merged 15 commits into
PlasmaPy:mainfrom
namurphy:particle-input-typing

Conversation

@namurphy

@namurphy namurphy commented Jan 4, 2024 •

Copy link
Copy Markdown
Member

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
@codecov

codecov Bot commented Jan 4, 2024 •

Copy link
Copy Markdown

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.

Comment thread plasmapy/particles/decorators.py Resolved
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
Copy Markdown
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
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤔

Comment thread plasmapy/particles/decorators.py Resolved
Comment thread plasmapy/particles/decorators.py Resolved
Comment on lines +533 to +534
def f(particle: ParticleLike) -> Union[Particle, CustomParticle, ParticleList]:
return particle # type: ignore[return-value]

Copy link
Copy Markdown
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
Comment thread plasmapy/particles/decorators.py Resolved
Comment thread plasmapy/particles/decorators.py Resolved
`dict` of `str` to `object`
"""
return self._data.get("annotations")
return self._data.get("annotations") # type: ignore[return-value]

Copy link
Copy Markdown
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.

@namurphy
namurphy requested a review from ejohnson-96 January 9, 2024 20:17

@ejohnson-96 ejohnson-96 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

Comment thread mypy.ini
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
Copy Markdown
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
Copy Markdown
Member Author

Thank you for the reviews, @ejohnson-96!

@namurphy
namurphy merged commit 6918db6 into PlasmaPy:main Jan 10, 2024
@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