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 for type checking #280

Closed
Saransh-cpp opened this issue Sep 25, 2024 · 3 comments
Closed

Add mypy for type checking #280

Saransh-cpp opened this issue Sep 25, 2024 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@Saransh-cpp
Copy link
Member

Is Your Feature Request Related to a Problem? Please Describe

It will be useful for both the users and the developers to have well defined static types for everything in glass. The repository right now does have types in places but they are not tested. These can be tested by adding mypy as a pre-commit hook. Satisfying mypy will also pave the way for experimenting with mypyc compilations.

Related issues - #187. #246

Describe the Solution You'd Like

No response

Describe Alternatives You've Considered

No response

Additional Context

No response

@Saransh-cpp Saransh-cpp added the enhancement New feature or request label Sep 25, 2024
paddyroddy added a commit that referenced this issue Oct 3, 2024
Closes #246. Use `numpy.typing.NDArray` as the first step towards #280.
With `NDArray` we can use types, i.e. `NDArray[np.float64]`, which I'll
save for future work (#306).

I have also gone throughout and made sure anything like `ArrayLike` is
using the alias `import numpy.typing as npt`. Imports are generally in a
`if TYPE_CHECKING` block where possible.

We are using `numpy.testing` in some places. This was being aliased as
`npt`. To avoid confusion, I have removed this alias and left them as an
explicit import.
@paddyroddy paddyroddy self-assigned this Oct 3, 2024
paddyroddy added a commit that referenced this issue Oct 3, 2024
@paddyroddy
Copy link
Member

Splitting this into two issues

@paddyroddy paddyroddy changed the title Add static types and use mypy for type checking Add mypy for type checking Oct 14, 2024
@paddyroddy
Copy link
Member

See #358

paddyroddy added a commit that referenced this issue Oct 15, 2024
Adding `mypy` to a pre-existing codebase is never easy. I initially
attempted this in #308, but have since split this up into separate
issues:
- [x] #347
- [x] #356
- [ ] #358

In this PR, `mypy` is added but in order for CI to pass, the `# type:
ignore[]` syntax is used throughout. I didn't want to tackle these here
too (see #308) as it gets quite messy.

One thing I have done (following #356) is change every empty
`npt.NDArray` to `npt.NDArray[typing.Any]` (see #330), as this actually
results in fewer errors than leaving them all blank. Ideally, we'd like
to fill in as many of the `typing.Any` as possible - they're a bit
useless by themselves. However, that is not the priority for now. Plus,
I expect typing to change when #67 is tackled.
@paddyroddy
Copy link
Member

Closing due to #359, in favour of #358

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants