Skip to content

Improve consistency in type hint annotations #572

Description

@namurphy

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

  1. added
    docsPlasmaPy Docs at http://docs.plasmapy.org
    refactoring ♻️Improving an implementation without adding new functionality
    status: needs discussionIssues & PRs that need to be discussed at a community meeting or by the Coordinating Committee
    on Oct 23, 2018
  2. namurphy commented on Oct 23, 2018

    @namurphy
    MemberAuthor

    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.MyClass and foo.bar.yourclass.YourClass.

  3. added
    good first issueGood first contributions for new contributors
    and removed
    status: needs discussionIssues & PRs that need to be discussed at a community meeting or by the Coordinating Committee
    on Oct 30, 2018
  4. namurphy commented on Oct 30, 2018

    @namurphy
    MemberAuthor

    There was general support for this in our community meeting today, so let's go for it and make these changes. 🙂

  5. Jac0bDeal commented on Nov 30, 2018

    @Jac0bDeal
    Contributor

    @namurphy I could jump on this, I'd like to get involved with the project :)

  6. Jac0bDeal commented on Nov 30, 2018

    @Jac0bDeal
    Contributor

    For type hints such as u.kg, u.K, or np.nan, should they be updated to match the consistency? I could see arguments going both ways, namely that the u and np in these cases actually increase readability by specifying the source.

  7. StanczakDominik commented on Nov 30, 2018

    @StanczakDominik
    Member

    @Jac0bDeal go for it! :) Let's definitely keep variables such as u.kg and np.nan the way they are, though - you're quite right about better readability there, there is such a thing as too short a variable name. :D

  8. Jac0bDeal commented on Nov 30, 2018

    @Jac0bDeal
    Contributor

    @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. :)

  9. StanczakDominik commented on Nov 30, 2018

    @StanczakDominik
    Member

    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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    docsPlasmaPy Docs at http://docs.plasmapy.orggood first issueGood first contributions for new contributorsrefactoring ♻️Improving an implementation without adding new functionality

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions