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

ENH: Add minimal type annotations and run mypy in CI #1115

Merged
merged 5 commits into from
Jan 6, 2023

Conversation

effigies
Copy link
Member

@effigies effigies commented Jun 16, 2022

This PR aims to get us started down the path of type annotations (#1109) by getting to a minimal working mypy check.

These annotations are not intended to be comprehensive, but resolved mypy errors in what seemed the most correct and succinct way.

Thanks to @simkarwin's #1152 I've learned that we can use from __future__ import annotations to use Python 3.10-style annotations in a code-base that supports Python 3.7, which is quite nice.

I've shielded the tests/ directories from type checking for now. Resolving issues there will probably help stress test our annotations, but it's a bit much for my current attention span.

@codecov
Copy link

codecov bot commented Jun 16, 2022

Codecov Report

Base: 92.38% // Head: 92.38% // No change to project coverage 👍

Coverage data is based on head (02c7a34) compared to base (cfa209a).
Patch has no changes to coverable lines.

❗ Current head 02c7a34 differs from pull request most recent head 85f4cb4. Consider uploading reports for the commit 85f4cb4 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1115   +/-   ##
=======================================
  Coverage   92.38%   92.38%           
=======================================
  Files          97       97           
  Lines       12250    12250           
  Branches     2525     2525           
=======================================
  Hits        11317    11317           
  Misses        613      613           
  Partials      320      320           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@effigies

This comment was marked as resolved.

@effigies effigies force-pushed the enh/type_annotations branch 2 times, most recently from 73a443b to 7cbc4d3 Compare January 2, 2023 06:22
@effigies effigies changed the title ENH: Add type annotations ENH: Add minimal type annotations and run mypy in CI Jan 2, 2023
@effigies effigies marked this pull request as ready for review January 2, 2023 06:26
@effigies effigies added this to the 5.0.0 milestone Jan 2, 2023
@effigies effigies force-pushed the enh/type_annotations branch 4 times, most recently from bf14770 to 83aa395 Compare January 2, 2023 16:49
@effigies
Copy link
Member Author

effigies commented Jan 2, 2023

If anybody's feeling typish, I would appreciate a review here.

@effigies effigies force-pushed the enh/type_annotations branch 4 times, most recently from 73da8b0 to e5b6a66 Compare January 3, 2023 17:51
@effigies
Copy link
Member Author

effigies commented Jan 6, 2023

I plan to merge this in the morning. If someone wants to review first but hasn't found time, let me know.

@effigies effigies merged commit a60d2b7 into master Jan 6, 2023
@effigies effigies deleted the enh/type_annotations branch January 6, 2023 16:11
@fepegar
Copy link
Contributor

fepegar commented Jan 10, 2023

This is great. Thanks, @effigies!

@fepegar
Copy link
Contributor

fepegar commented Feb 11, 2023

I'm still getting Skipping analyzing "nibabel": module is installed, but missing library stubs or py.typed marker for the latest release. Do you need a py.typed file?

@fepegar
Copy link
Contributor

fepegar commented Feb 11, 2023

Happy to contribute, if you do.

@effigies
Copy link
Member Author

Please do!

@effigies
Copy link
Member Author

Fernando, I went ahead and opened #1196.

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

Successfully merging this pull request may close these issues.

2 participants