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

Mypy Update #731

Merged

Conversation

daralynnrhode
Copy link
Contributor

@daralynnrhode daralynnrhode commented Aug 6, 2024

Overview

Kinda long, so sorry for that. Feel free to only check the files that concern your instrument.

Ensuring that both pre-commit mypy and a local mypy check will return the same outputs. In doing so, more errors were found and will be fixed within this PR.

numpy is weird:
As a way to avoid redeclaring a type, ie. indices: np.ndarray = np.where(valid)[0] that should be inferred to be a np.ndarray because of what the np.where() returns, but is not. Instead function returns of np.ndarray were changed to npt.NDArray from the numpy typing library. I believe in most instance it will represent the same thing.
This did however cause the build the docs to fail to recognize the type for some functions, but not all. Just added an ignore to the config.py file.

*np.array is not an object type, it is a function that returns an np.ndarray. object: np.array = is not valid.

Closes #710

New Dependencies

New Files

Updated Files

  • pyproject.toml

  • pre-commit-config.yaml

  • docs/source/conf.py

Testing

@daralynnrhode daralynnrhode self-assigned this Aug 6, 2024
Copy link
Collaborator

@greglucas greglucas left a comment

Choose a reason for hiding this comment

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

🎉 This is fantastic! Thank you for updating all of this and the persistence to clean it all up.

I have a few minor nitpicks where I don't like what the type checker made you do and added some suggestions for what I think might be easier/better.

imap_processing/cdf/utils.py Outdated Show resolved Hide resolved
imap_processing/swapi/l1/swapi_l1.py Outdated Show resolved Hide resolved
imap_processing/swapi/l1/swapi_l1.py Outdated Show resolved Hide resolved
imap_processing/swe/l1a/swe_science.py Outdated Show resolved Hide resolved
imap_processing/swe/l1a/swe_science.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.

Just a few non-blocking comments/suggestions from me. Thank you for doing this work -- you really had to get into the weeds in this one! It is much appreciated!

imap_processing/mag/l0/mag_l0_data.py Outdated Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
@daralynnrhode daralynnrhode changed the title [WIP] Mypy Update Mypy Update Aug 8, 2024
@daralynnrhode daralynnrhode requested a review from greglucas August 8, 2024 16:15
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.

Looks good, thanks for doing this!

@@ -109,7 +109,9 @@ def __post_init__(self) -> None:
# Convert string output from space_packet_parser to numpy array of
# big-endian bytes
self.VECTORS = np.frombuffer(
int(self.VECTORS, 2).to_bytes(len(self.VECTORS) // 8, "big"),
int(self.VECTORS, 2).to_bytes(len(self.VECTORS) // 8, "big"), # type: ignore[arg-type]
# TODO Check MYPY Error: Argument 1 to "int" has incompatible type
Copy link
Contributor

Choose a reason for hiding this comment

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

I can check on this, I'm updating things related to this code soon

@daralynnrhode daralynnrhode merged commit e6908f8 into IMAP-Science-Operations-Center:dev Aug 9, 2024
17 checks passed
@daralynnrhode daralynnrhode deleted the mypy_update branch August 9, 2024 13:28
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.

BUG - mypy should only be run over our actual code
4 participants