Repository navigation
Improve consistency in type hint annotations #572
Description
Right now, we have a mix of type hint annotation styles. Sometimes we import the packages:
import typing
import numbers
def f(x: typing.Optional[numbers.Real]):
...Other times, we import the classes from the packages:
from typing import Optional
from numbers import Real
def f(x: Optional[Real]):
...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.
Activity
namurphy commentedon Oct 23, 2018
Also, from the PEP 8 style guide discussion of imports:
namurphy commentedon Oct 30, 2018
There was general support for this in our community meeting today, so let's go for it and make these changes. 🙂
Jac0bDeal commentedon Nov 30, 2018
@namurphy I could jump on this, I'd like to get involved with the project :)
Jac0bDeal commentedon Nov 30, 2018
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.StanczakDominik commentedon Nov 30, 2018
@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. :DJac0bDeal commentedon Nov 30, 2018
@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. :)
StanczakDominik commentedon Nov 30, 2018
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 :)