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] start using mypy and updating types #86

Merged
merged 23 commits into from
Mar 25, 2024
Merged

Conversation

Remi-Gau
Copy link
Collaborator

@Remi-Gau Remi-Gau commented Dec 15, 2023


  • also applied isort to the imports

TODO:

  • try to be more specific and remove most Any (probably in a follow up PR)

@codecov-commenter
Copy link

codecov-commenter commented Dec 15, 2023

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (22a4ae0) 92.40% compared to head (b9a59c1) 91.84%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #86      +/-   ##
==========================================
- Coverage   92.40%   91.84%   -0.56%     
==========================================
  Files          11       11              
  Lines         487      503      +16     
==========================================
+ Hits          450      462      +12     
- Misses         37       41       +4     
Files Coverage Δ
giga_connectome/__init__.py 81.81% <100.00%> (ø)
giga_connectome/connectome.py 94.87% <100.00%> (+0.13%) ⬆️
giga_connectome/mask.py 97.00% <100.00%> (+0.03%) ⬆️
giga_connectome/run.py 96.55% <100.00%> (+0.25%) ⬆️
giga_connectome/workflow.py 90.38% <100.00%> (ø)
giga_connectome/postprocess.py 89.70% <90.90%> (ø)
giga_connectome/atlas.py 85.48% <91.30%> (+0.02%) ⬆️
giga_connectome/denoise.py 84.78% <77.77%> (-3.32%) ⬇️
giga_connectome/utils.py 93.42% <85.18%> (-1.25%) ⬇️

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not sure it makes sense to check for type annotation in tooling scripts...

@Remi-Gau Remi-Gau changed the title [WIP][ENH] start using mypy and updating types [ENH] start using mypy and updating types Dec 15, 2023
@Remi-Gau Remi-Gau marked this pull request as ready for review December 15, 2023 12:05
@Remi-Gau
Copy link
Collaborator Author

@htwangtw
I had to amend more than just the types because the code was in places doing things that did not match the types, so this turns into a lot more line changes than I had anticipated.

Let me know if you have questions.

Copy link
Collaborator

@htwangtw htwangtw left a comment

Choose a reason for hiding this comment

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

I certainly learned a lot reading this PR and thank you for doing this!!!

group_mask: Union[str, Path, Nifti1Image],
atlas_image: Union[str, Path, Nifti1Image],
) -> Tuple[np.ndarray, np.ndarray]:
correlation_matrix: np.ndarray[Any, Any],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this how to declare the dimension of the numpy array? That's cool

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I need to investigate better how to use types with numpy, this is a bit of a brute force approach.

@Remi-Gau
Copy link
Collaborator Author

Remi-Gau commented Jan 8, 2024

I certainly learned a lot reading this PR

To be honest I had to learn a lot to make it happen!!!

@Remi-Gau
Copy link
Collaborator Author

merge conflict resolved

let's see what CI says

@Remi-Gau
Copy link
Collaborator Author

merge conflicts resolved

Copy link
Collaborator

@htwangtw htwangtw left a comment

Choose a reason for hiding this comment

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

LGTM and this is really cool. Merging....

@htwangtw htwangtw merged commit 558607f into bids-apps:main Mar 25, 2024
15 checks passed
@Remi-Gau Remi-Gau deleted the mypy branch April 12, 2024 06:19
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.

use mypy to check type hints?
3 participants