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

Add type stub file for @wrapt.decorator #2442

Merged
merged 4 commits into from Jan 4, 2024

Conversation

namurphy
Copy link
Member

@namurphy namurphy commented Jan 3, 2024

Description

This PR adds a type stub file for wrapt.

Motivation and context

A few of our widely used decorators like @particle_input make use of @wrapt.decorator. Because wrapt is untyped, we were running into issues with mypy not type checking some of our code because @wrapt.decorator is untyped. Adding a type stub file lets us get past that. This also serves as a prototype type stub file.

Related issues

This came up in #2429.

Copy link

codecov bot commented Jan 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1c70047) 96.92% compared to head (d33475b) 96.92%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2442   +/-   ##
=======================================
  Coverage   96.92%   96.92%           
=======================================
  Files         104      104           
  Lines        9133     9153   +20     
=======================================
+ Hits         8852     8872   +20     
  Misses        281      281           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@namurphy namurphy changed the title Add type stubs file for wrapt Add type stub file for wrapt Jan 3, 2024
@namurphy namurphy changed the title Add type stub file for wrapt Add type stub file for @wrapt.decorator Jan 3, 2024
@namurphy namurphy marked this pull request as ready for review January 3, 2024 22:10
@namurphy namurphy requested a review from a team as a code owner January 3, 2024 22:10
@namurphy namurphy requested review from ejohnson-96 and pheuer and removed request for a team January 3, 2024 22:10
Comment on lines 5 to 9
exclude = (?x)(
docs|
\.run|
\.tox
/docs/
| /\.run/
| /\.tox/
)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This corrects the paths to exclude when doing static type checking with mypy.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent of the new stub file, correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, this is an independent change.

@@ -68,9 +66,6 @@ ignore_errors = true
# gradually make PlasmaPy consistent with mypy's strict mode, we can
# remove the disabled error codes file-by-file.

Copy link
Member Author

@namurphy namurphy Jan 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the changes in this file made below this line were done automatically, as described in #2424.

This PR is removing a lot of the mypy errors that we had been ignoring, while also uncovering some new errors for code that wasn't type checked because it depended on @wrapt.decorator in some way.

Copy link
Member

@pheuer pheuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good - are there any other dependencies with the same problem/solution?

@namurphy
Copy link
Member Author

namurphy commented Jan 4, 2024

Thank you for the review!

Sounds good - are there any other dependencies with the same problem/solution?

Yes. Several. The most pressing and most difficult will be astropy.units, which is being discussed in astropy/astropy#15808. I don't want to permanently add a type stub file for astropy.units here, since it would make a lot more sense to add that in Astropy itself. We might add a stub file here temporarily, if it gets added in Astropy but won't be included in a release for a while.

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

Successfully merging this pull request may close these issues.

None yet

2 participants