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
Update type hints and docstrings in plasmapy.formulary
#2543
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2543 +/- ##
==========================================
+ Coverage 96.93% 97.35% +0.42%
==========================================
Files 104 104
Lines 9163 9389 +226
==========================================
+ Hits 8882 9141 +259
+ Misses 281 248 -33 ☔ View full report in Codecov by Sentry. |
@@ -18,7 +18,7 @@ | |||
from plasmapy import particles | |||
from plasmapy.formulary.collisions import coulomb, lengths, misc | |||
from plasmapy.formulary.speeds import thermal_speed | |||
from plasmapy.particles import ParticleLike | |||
from plasmapy.particles.particle_class import ParticleLike |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a mypy error about how plasmapy.particles
doesn't explicitly export ParticleLike
. Hence, I'm making a bunch of these import statements a lot more specific.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I remember this issue
@particles.particle_input | ||
@particle_input | ||
def impact_parameter_perp( | ||
T: u.Quantity[u.K], | ||
species: (particles.Particle, particles.Particle), | ||
species: (Particle, Particle), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be more consistent with the import conventions we use elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love it when a review is just basically import statements, it makes the review a lot easier 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes the pull request a lot easier too!
"RotatingTensorElements", | ||
"StixTensorElements", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I needed to expose these to the public API so that they could be used as return annotations without causing a "missing reference" error when doing the doc build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain this when we next talk please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be happy to, with the caveat that I only understand 83% of what's happening here!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
This PR adds and updates a bunch of type hint annotations in
plasmapy.formulary
. It also makes a few minor refactoring changes.A limiting factor in doing this is that
@validate_quantities
is currently largely unannotated (#2435), but is being used to decorate a whole bunch of different functions. However, I'm going to leave that for another day (or more likely, another month).