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 type annotations #281

Open
1 task done
schwehr opened this issue Sep 1, 2024 · 4 comments
Open
1 task done

Add type annotations #281

schwehr opened this issue Sep 1, 2024 · 4 comments

Comments

@schwehr
Copy link
Contributor

schwehr commented Sep 1, 2024

Describe the feature request

Once the code on the main branch is only for python >= 3.9, it would be great from a user pov to add some type annotations. I find them really helpful when writing code using a library (both as a human and for autocomplete). Also, they tend for find weird corner case bugs in existing code.

I can do initial work on type annotations for the easy cases.

Contributions

  • I am interested in implementing the described feature request and submit as a PR.
@JamesParrott
Copy link
Collaborator

This would be really good, especially given the frequency PyShp decodes bytes to str and encodes str to bytes.

Which type checker should we use?

@schwehr
Copy link
Contributor Author

schwehr commented Sep 8, 2024

Which type checker should we use?

It's easy enough to have a GitHub action use the 3 major ones. And mypy would be a likely good default for the primary one.

earthengine-api has an action that checks all 3

https://github.com/google/earthengine-api/blob/master/.github%2Fworkflows%2Fci-type-verification.yml

@JamesParrott
Copy link
Collaborator

Sounds great. Kinda like checking your C++ compiles with clang, gcc and msvc.

I would just point out this is essentially a mature code base (albeit only a ~2000 line file), so don't torture yourself. I want to encourage yourself, and other new contributors, and PyShp is exactly the sort of library that will benefit from static typing.

But if you encounter a fiendish typing challenge, that feels like you're fighting the Typescript compiler to no avail, then it's fine with me to type it as Any, and perhaps add it as a Todo issue.

The Python typing system is still under development, and an algebraic typing system in particular is being worked on. So for some typing problems, it might be simply best to wait a year. After which there may be a much better way to solve it.

@JamesParrott
Copy link
Collaborator

JamesParrott commented Sep 15, 2024

@schwehr I've added some basic type hints,

Point_T = Sequence[float]

and have set up dmypy in CI via pre-commit. Ruff and Ruff-format too.

New contributors should be able to install pre-commit themselves, and run pre-commit install to install and run the hooks locally. Dmypy should be a bit faster on repeated runs, after the daemon spins up.

  • Once all functions have type annotations, --strict can be added to the dmypy args

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants