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

Fix mypy errors #914

Merged
merged 7 commits into from
Jul 4, 2022
Merged

Fix mypy errors #914

merged 7 commits into from
Jul 4, 2022

Conversation

fepegar
Copy link
Owner

@fepegar fepegar commented Jun 26, 2022

Related to #810.

Description
This pull request addresses many of the mypy errors in the library. Next should be:

  • Run mypy in CI
  • Annotating the tests
  • Make mypy pass even with the --strict flag

Checklist

  • I have read the CONTRIBUTING docs and have a developer setup (especially important are pre-commitand pytest)
  • Non-breaking change (would not break existing functionality)
  • Breaking change (would cause existing functionality to change)
  • Tests added or modified to cover the changes
  • Integration tests passed locally by running pytest
  • In-line docstrings updated
  • Documentation updated, tested running make html inside the docs/ folder
  • This pull request is ready to be reviewed
  • If the PR is ready and there are multiple commits, I have squashed them and force-pushed

@codecov
Copy link

codecov bot commented Jun 26, 2022

Codecov Report

Merging #914 (96f6110) into main (1e7d9e9) will increase coverage by 0.15%.
The diff coverage is 93.52%.

@@            Coverage Diff             @@
##             main     #914      +/-   ##
==========================================
+ Coverage   86.40%   86.56%   +0.15%     
==========================================
  Files          91       91              
  Lines        5326     5448     +122     
==========================================
+ Hits         4602     4716     +114     
- Misses        724      732       +8     
Impacted Files Coverage Δ
src/torchio/transforms/intensity_transform.py 100.00% <ø> (ø)
src/torchio/utils.py 75.22% <33.33%> (-0.24%) ⬇️
src/torchio/data/io.py 92.00% <53.33%> (-1.48%) ⬇️
src/torchio/datasets/bite.py 33.33% <66.66%> (ø)
src/torchio/datasets/visible_human.py 63.15% <75.00%> (+2.04%) ⬆️
src/torchio/transforms/transform.py 94.09% <82.35%> (-0.33%) ⬇️
src/torchio/data/sampler/grid.py 88.46% <85.71%> (+0.14%) ⬆️
...o/transforms/augmentation/intensity/random_swap.py 96.39% <96.77%> (-0.64%) ⬇️
src/torchio/data/dataset.py 100.00% <100.00%> (ø)
src/torchio/data/image.py 87.95% <100.00%> (+0.32%) ⬆️
... and 40 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@fepegar
Copy link
Owner Author

fepegar commented Jun 26, 2022

@jcreinhold, I know this is a huge PR, but could you please quickly skim through it and approve it if it looks good to you?

@fepegar
Copy link
Owner Author

fepegar commented Jun 26, 2022

Ok, I think I messed up and have made things not compatible with Python < 3.10. I'll have to turn this into a draft and work more on this PR.

@fepegar fepegar marked this pull request as draft June 26, 2022 21:42
@fepegar fepegar force-pushed the deal-with-mypy branch 4 times, most recently from 0ee3be8 to c93d303 Compare June 27, 2022 00:27
Fix some mypy errors

Fix some mypy errors

Fix some mypy errors

Fix some flake8 errors

Fix some mypy errors

Fix some mypy errors

Fix some mypy errors

Fix tests

Edit pre-commit config

[pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci
@fepegar fepegar marked this pull request as ready for review June 27, 2022 07:39
@jcreinhold
Copy link
Contributor

Hey @fepegar I just started a new job and am pretty busy. I'll try to review the PR this weekend, but, if you don't hear from me by the end of this weekend (July 3), I might be too pressed for time to meaningfully review this.

@fepegar fepegar merged commit 9150afc into main Jul 4, 2022
@fepegar fepegar deleted the deal-with-mypy branch July 4, 2022 22:02
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.

None yet

2 participants