Repository navigation
Improve consistency in type hint annotations #572
Description
Activity
- addeddocsPlasmaPy Docs at http://docs.plasmapy.orgPlasmaPy Docs at http://docs.plasmapy.orgrefactoring ♻️Improving an implementation without adding new functionalityImproving an implementation without adding new functionalitystatus: needs discussionIssues & PRs that need to be discussed at a community meeting or by the Coordinating CommitteeIssues & PRs that need to be discussed at a community meeting or by the Coordinating Committee
on Oct 23, 2018 Also, from the PEP 8 style guide discussion of imports:
When importing a class from a class-containing module, it's usually okay to spell this:
from myclass import MyClass from foo.bar.yourclass import YourClass
If this spelling causes local name clashes, then spell them explicitly:
import myclass import foo.bar.yourclass
and use
myclass.MyClassandfoo.bar.yourclass.YourClass.- addedgood first issueGood first contributions for new contributorsGood first contributions for new contributorsand removedstatus: needs discussionIssues & PRs that need to be discussed at a community meeting or by the Coordinating CommitteeIssues & PRs that need to be discussed at a community meeting or by the Coordinating Committee
on Oct 30, 2018 There was general support for this in our community meeting today, so let's go for it and make these changes. 🙂
@namurphy I could jump on this, I'd like to get involved with the project :)
For type hints such as
u.kg,u.K, ornp.nan, should they be updated to match the consistency? I could see arguments going both ways, namely that theuandnpin these cases actually increase readability by specifying the source.@Jac0bDeal go for it! :) Let's definitely keep variables such as
u.kgandnp.nanthe way they are, though - you're quite right about better readability there, there is such a thing as too short a variable name. :D@StanczakDominik Cool, thanks! I've also noted some missing type hints, but I'm leaving them untouched because I'd rather lean towards keeping the PR on just this topic. Should I make note of the missing type hints and create a new issue for adding them?
Also thanks for the quick reply! I'm excited to join the project as I haven't gotten to work on much physics since graduating a couple years back. :)
Either a new issue or going a little bit out of issue scope along the way is completely fine. 😃
The pleasure is mine! It's nice to see you're interested :)
Right now, we have a mix of type hint annotation styles. Sometimes we import the packages:
Other times, we import the classes from the packages:
Importing the packages has the advantage of showing the full namespace. Importing the classes from the packages has the advantage of improving readability.
I propose that we standardize our type hints by using the second example above and importing the classes from the packages before using them in annotations. I prefer the second method because it prioritizes readability, and there's very little chance of local name clashes. Most of the time, the annotations are only being used to annotate (with the exception of annotations like
Optional[Particle]when used with@particle_input).Whichever we decide, we should apply this consistently throughout PlasmaPy, and describe the preferred import style in the development guide.