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 mypy Pre-Commit Hook for files Pt.2 #661

Merged

Conversation

daralynnrhode
Copy link
Contributor

@daralynnrhode daralynnrhode commented Jun 25, 2024

Adding the mypy Pre-commit hook for non-instrument files and folders

Overview

Changes for this PR will occur in the imap_processing files and imap_processing/cdf Applying the mypy pre-commit and making any necessary changes to then pass the checks.

Please double check all function definitions and the types applied to the parameters.
With any TODO listed I tried to explain what error it was causing and then ignored the error in the code line.

The places where inline ignore comments are will be changed/fixed before the PR is merged.

New Dependencies

New Files

Deleted Files

Updated Files

-imap_processing/utils.py
-imap_processing/decom.py
-imap_processing/cli.py
-imap_processing/cdf/utils.py
-imap_processing/cdf/imap_cdf_manager.py
-imap_processing/cdf/cdf_attribute_manager.py

Testing

@daralynnrhode daralynnrhode self-assigned this Jun 25, 2024
@daralynnrhode daralynnrhode added Repo: Documentation Improvements or additions to documentation Repo: Testing Related to testing labels Jun 25, 2024
Copy link
Contributor

@maxinelasp maxinelasp left a comment

Choose a reason for hiding this comment

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

Nice! I had some comments but great work so far!

imap_processing/cdf/cdf_attribute_manager.py Show resolved Hide resolved
imap_processing/cdf/utils.py Outdated Show resolved Hide resolved
imap_processing/cli.py Show resolved Hide resolved
imap_processing/cli.py Outdated Show resolved Hide resolved
imap_processing/cli.py Outdated Show resolved Hide resolved
imap_processing/cli.py Outdated Show resolved Hide resolved
imap_processing/cli.py Outdated Show resolved Hide resolved
imap_processing/cli.py Outdated Show resolved Hide resolved
imap_processing/cli.py Outdated Show resolved Hide resolved
imap_processing/utils.py Outdated Show resolved Hide resolved
@daralynnrhode daralynnrhode changed the title [WIP] Add mypy Pre-Commit Hook for files Add mypy Pre-Commit Hook for files Jun 25, 2024
@daralynnrhode daralynnrhode requested review from a team, bourque, sdhoyt, greglucas, subagonsouth, tech3371, vmartinez-cu and anamanica and removed request for a team June 25, 2024 18:01
@daralynnrhode daralynnrhode changed the title Add mypy Pre-Commit Hook for files Add mypy Pre-Commit Hook for files Pt.2 Jun 26, 2024
Copy link
Collaborator

@bourque bourque left a comment

Choose a reason for hiding this comment

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

Nice work here, especially with these weird corner cases. I have a few suggestions to fix things, otherwise this looks good!

imap_processing/cdf/cdf_attribute_manager.py Outdated Show resolved Hide resolved
imap_processing/cdf/cdf_attribute_manager.py Outdated Show resolved Hide resolved
imap_processing/cdf/utils.py Outdated Show resolved Hide resolved
imap_processing/cdf/utils.py Outdated Show resolved Hide resolved
imap_processing/cli.py Outdated Show resolved Hide resolved
imap_processing/cli.py Outdated Show resolved Hide resolved
imap_processing/cli.py Outdated Show resolved Hide resolved
imap_processing/cli.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@bourque bourque left a comment

Choose a reason for hiding this comment

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

Thanks for making those changes. We are ignoring some errors here but I think they are for valid reasons. So looks good to me!

@daralynnrhode daralynnrhode merged commit 1246b6e into IMAP-Science-Operations-Center:dev Jul 4, 2024
17 checks passed
@daralynnrhode daralynnrhode deleted the add_mypy_files branch July 4, 2024 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Repo: Documentation Improvements or additions to documentation Repo: Testing Related to testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants