search

GeospatialPython / pyshp

This library reads and writes ESRI Shapefiles in pure Python.
MIT License
1.11k stars 260 forks source link

Add type annotations #281

Open schwehr opened 2 months ago

schwehr commented 2 months ago

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

JamesParrott commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 1 month ago

@schwehr I've added some basic type hints, https://github.com/GeospatialPython/pyshp/blob/8015acc782e3ff6d4eee2b88005b4c1190d2a593/shapefile.py#L151 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.