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

[WIP] Mypy Check Bug Fix #713

Closed

Conversation

daralynnrhode
Copy link
Contributor

@daralynnrhode daralynnrhode commented Jul 25, 2024

A fix for the mypy bug and any needed changes

Overview

Closes #710

Ensures that mypy will only check files within imap_processing/ etc. Changes to make the pre-commit mypy check and regular mypy check align.

Currently, I am being pushed in the direction to think it is a recursion difference that is causes the difference in errors for the two checks. I am trying to track down where this is occurring.

For Reference:

mypy . ran from imap_processing/ produces
Screen Shot 2024-07-25 at 9 53 58 AM

pre-commit run --all ran from imap_processing/ produces
Screen Shot 2024-07-25 at 9 55 29 AM

To Note; all errors found by running the pre-check are present when the mypy check is ran by itself

New Dependencies

New Files

Deleted Files

Updated Files

  • pyproject.toml
  • .pre-commit-config.yaml
  • codice_l1a.py

Testing

@daralynnrhode daralynnrhode self-assigned this Jul 25, 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.

Looks like you need to merge or rebase on upstream/dev to get the latest changes before the tests can run.

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
@daralynnrhode
Copy link
Contributor Author

With the most recent commit I am still having errors produced when just mypy is ran.

.pre-commit-config.yaml Outdated Show resolved Hide resolved
@daralynnrhode
Copy link
Contributor Author

daralynnrhode commented Jul 25, 2024

Now back to the point of pre-commit passing but the local mypy check produces errors. I am hedging toward this being caused because of a difference in the recursive calls for mypy when it is called individually versus in pre-commit.

In the mypy documentation it is stated that submodules found in what is listed in packages will be checked, so the pyproject is covering all files within imap_processing and tools. Shouldn't the pre-commit-when it runs the mypy check-check the pyproject for how mypy should be configured?

@daralynnrhode
Copy link
Contributor Author

Turns out since the pre-commit runs its check in an isolated virtualenv some things may be lost.
https://github.com/pre-commit/mirrors-mypy

I believe the dependencies need to be listed for mypy but am at a loss as to what all those dependencies should be.

Comment on lines 302 to 306
dtype_array: np.ndarray = np.array(values, dtype=dtype)
return dtype_array
except ValueError:
return np.array(values)
regular_array: np.ndarray = np.array(values)
return regular_array
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that for all of the numpy typing we need to import their typing module, and then use npt.NDArray. Saying that we are returning np.ndarray seems incorrect, so I think we should change those, rather than add these explicit type annotations throughout the code.

import numpy.typing as npt

def func() -> npt.NDArray

https://stackoverflow.com/questions/66349242/specific-type-annotation-for-numpy-ndarray-using-mypy

@daralynnrhode daralynnrhode deleted the mypy_bug branch August 6, 2024 15:58
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
2 participants