Skip to content
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

Use mypy for static type checking in continuous integration #268

Closed
namurphy opened this issue Feb 17, 2018 · 3 comments · Fixed by #2424 or #2432
Closed

Use mypy for static type checking in continuous integration #268

namurphy opened this issue Feb 17, 2018 · 3 comments · Fixed by #2424 or #2432
Labels
CI Related to continuous integration Plasma Lv0 | Novice Issues that do not require knowledge of physics Python Lv3 | Proficient Issues that require proficiency in Python revisit in 2024 Issues that we should revisit in ∼2024 static type checking testing upstream fix required Issues & PRs that are blocked due to a problem with a dependency
Milestone

Comments

@namurphy
Copy link
Member

Mypy uses function annotations to perform static type checking. Mypy has the potential to help us track down bugs without even running a program. Lately we've started to add type hints to functions and methods in a few of our subpackages (including atomic, for example) where we sometimes make use of the typing package as well. It would be great if we can use mypy in our suite of tests with Travis CI which would involve updating .travis.yml (which is also being edited in #264).

A potential problem is that some return annotations are given as units:

def func() -> u.eV:

I am not sure how mypy will handle that situation. Some function arguments are going to be annotated with the Particle class in order for @particle_input to know what to do with them. That may also make mypy grumpy. Another thing to worry about is that backwards compatibility is not guaranteed for mypy yet, so we might have to make some further adjustments later on. I personally am inexperienced with mypy, but projects such as zulip have added mypy to their Travis CI tests.

Many thanks to whoever ends up working on this! (And also thanks to @Cadair for letting us know about mypy!)

@namurphy namurphy added effort: medium Requiring perhaps ∼1–3 days priority: medium Issues & PRs of moderate urgency and importance help wanted Unassigned issues that would benefit from assistance or contributions needed from the community testing Needs CI magic labels Feb 17, 2018
@namurphy namurphy added this to the v0.2 milestone Feb 17, 2018
@ghost ghost mentioned this issue Oct 2, 2018
@StanczakDominik
Copy link
Member

Probably best to close this one as we hadn't considered mypy and astropy.units clashing?

@namurphy namurphy added upstream fix required Issues & PRs that are blocked due to a problem with a dependency Snoozed and removed effort: medium Requiring perhaps ∼1–3 days Hacktoberfest help wanted Unassigned issues that would benefit from assistance or contributions needed from the community priority: medium Issues & PRs of moderate urgency and importance labels Oct 3, 2018
@namurphy
Copy link
Member Author

namurphy commented Oct 3, 2018

I just created a "Snoozed" label to add to this since we could potentially address this in the future, but there doesn't seem to be a viable way to address it now. I'll try to remember to create an issue on Astropy's repository that discusses this problem.

@namurphy namurphy reopened this Jan 31, 2022
@namurphy namurphy changed the title Use mypy for static type checking in Travis CI Use mypy for static type checking in continuous integration Jan 31, 2022
@namurphy namurphy removed the upstream fix required Issues & PRs that are blocked due to a problem with a dependency label Jan 31, 2022
@namurphy
Copy link
Member Author

Re-opening this because there now exists a workaround for Quantity type annotations that preserve units (see #1413).

@namurphy namurphy added Python Lv3 | Proficient Issues that require proficiency in Python Python Lv4 | Expert Issues that require in-depth knowledge of Python and removed Python Lv4 | Expert Issues that require in-depth knowledge of Python labels Aug 18, 2022
@namurphy namurphy added CI Related to continuous integration Plasma Lv0 | Novice Issues that do not require knowledge of physics upstream fix required Issues & PRs that are blocked due to a problem with a dependency labels May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Related to continuous integration Plasma Lv0 | Novice Issues that do not require knowledge of physics Python Lv3 | Proficient Issues that require proficiency in Python revisit in 2024 Issues that we should revisit in ∼2024 static type checking testing upstream fix required Issues & PRs that are blocked due to a problem with a dependency
Projects
None yet
2 participants