Parcels typing and mypy support #1653

VeckoTheGecko · 2024-08-12T16:13:42Z

Adding type annotations in the codebase will improve our documentation, improve linting (where users can hover over and quickly see types of variables), as well as allow for typechecking throughmypyPyright (should we implement it).

I was initially hesitant for this because I remembered the python community as a whole seems very split on this (and I also had some difficulties using mypy in the past). Revisiting it, I've seen some strong support especially with newer Python versions. I have also found out that https://github.com/python/typeshed provides "stubs" providing types for untyped packages.

Considerations:

[DONE in Enable Ruff formatting #1655] convert all files (at least in public API) to use Ruff formatting
- This will make adding types result in much easier to read git diffs as the formatting of parameters will be changed to be one per line
reduce code/doc duplication as much as possible
- ^^ This one will need some refactoring, as there is already quite some duplication in function parameter names (i.e., parameters which are taken as explicit input and propagated through to another function. This is seen in a few class constructors) and I don't want this to exacerbate it. Needs investigation on best way forward, and this bullet should probably be done in a separate PR for easy review.
- class methods in fieldset.py are a culprit
have a parcels.typing module to store type aliases commonly used in the codebase (e.g., for dimension mapping dictionaries which are of an expected format)
- Include this module in API documentation with explanation
enable pydoclint rules to make sure that annotated variables match the docstring. Ignore specific rules if need be
rely on the typing module and the types provided in there

Related: #1324

EDIT: Pyright was investigated. Seems that its stricter than mypy. At the moment just opting for mypy. See some discussion for more context

The text was updated successfully, but these errors were encountered:

VeckoTheGecko · 2024-08-13T08:33:32Z

@erikvansebille configuring Ruff at the moment. Default line length is 88, thoughts on having it be 120?

Some discussion on line length.

I think 120 would be good to avoid re-wrapping large conditionals across multiple lines, which we seem to have some of. If we want we can work to reducing this in future.

120 line width -> 1228 lines added
88 line width -> 4295 lines added

You can visually see what this looks like by adding a ruler in VScode:

Settings (cmd + ,)
Search Editor: Rulers and add one for 120

Overviews

120 line length "git diff --stat"

 docs/conf.py                                       |   87 +-
 docs/examples/example_brownian.py                  |   20 +-
 docs/examples/example_dask_chunk_OCMs.py           |  147 +-
 docs/examples/example_decaying_moving_eddy.py      |   33 +-
 docs/examples/example_globcurrent.py               |   84 +-
 docs/examples/example_mitgcm.py                    |    8 +-
 docs/examples/example_moving_eddies.py             |   52 +-
 docs/examples/example_nemo_curvilinear.py          |    4 +-
 docs/examples/example_ofam.py                      |   20 +-
 docs/examples/example_peninsula.py                 |   40 +-
 docs/examples/example_radial_rotation.py           |   16 +-
 docs/examples/example_stommel.py                   |   44 +-
 .../application_kernels/EOSseawaterproperties.py   |  158 ++-
 parcels/application_kernels/TEOSseawaterdensity.py |  121 +-
 parcels/application_kernels/advection.py           |  219 +--
 parcels/application_kernels/advectiondiffusion.py  |    7 +-
 parcels/application_kernels/interaction.py         |   15 +-
 parcels/compilation/codecompiler.py                |   56 +-
 parcels/compilation/codegenerator.py               |  362 +++--
 parcels/field.py                                   | 1478 ++++++++++++--------
 parcels/fieldfilebuffer.py                         |  374 +++--
 parcels/fieldset.py                                |  601 +++++---
 parcels/grid.py                                    |  365 +++--
 parcels/gridset.py                                 |    8 +-
 parcels/interaction/interactionkernel.py           |   94 +-
 parcels/interaction/neighborsearch/base.py         |   45 +-
 parcels/interaction/neighborsearch/basehash.py     |   22 +-
 parcels/interaction/neighborsearch/hashflat.py     |   21 +-
 .../interaction/neighborsearch/hashspherical.py    |   73 +-
 parcels/kernel.py                                  |  242 ++--
 parcels/particle.py                                |   72 +-
 parcels/particledata.py                            |  151 +-
 parcels/particlefile.py                            |  160 ++-
 parcels/particleset.py                             |  450 ++++--
 parcels/rng.py                                     |   14 +-
 parcels/tools/converters.py                        |   98 +-
 parcels/tools/exampledata_utils.py                 |    8 +-
 parcels/tools/global_statics.py                    |    4 +-
 parcels/tools/interpolation_utils.py               |  163 +--
 parcels/tools/statuscodes.py                       |   44 +-
 parcels/tools/timer.py                             |   20 +-
 pyproject.toml                                     |   93 +-
 tests/test_advection.py                            |  449 +++---
 tests/test_data/create_testfields.py               |   78 +-
 tests/test_diffusion.py                            |   75 +-
 tests/test_fieldset.py                             |  999 +++++++------
 tests/test_fieldset_sampling.py                    |  686 +++++----
 tests/test_grids.py                                |  614 ++++----
 tests/test_interaction.py                          |  167 ++-
 tests/test_kernel_execution.py                     |  169 +--
 tests/test_kernel_language.py                      |  361 ++---
 tests/test_mpirun.py                               |   34 +-
 tests/test_particlefile.py                         |  209 +--
 tests/test_particles.py                            |   76 +-
 tests/test_particlesets.py                         |  258 ++--
 tests/test_tools.py                                |    2 +-
 56 files changed, 5749 insertions(+), 4521 deletions(-)

88 line length "git diff --stat"

 docs/conf.py                                       |   86 +-
 .../application_kernels/EOSseawaterproperties.py   |  236 ++-
 parcels/application_kernels/TEOSseawaterdensity.py |  127 +-
 parcels/application_kernels/advection.py           |  384 ++--
 parcels/application_kernels/advectiondiffusion.py  |   59 +-
 parcels/application_kernels/interaction.py         |   21 +-
 parcels/compilation/codecompiler.py                |  126 +-
 parcels/compilation/codegenerator.py               |  611 ++++--
 parcels/field.py                                   | 2054 ++++++++++++++------
 parcels/fieldfilebuffer.py                         |  538 +++--
 parcels/fieldset.py                                |  819 +++++---
 parcels/grid.py                                    |  483 +++--
 parcels/gridset.py                                 |   12 +-
 parcels/interaction/interactionkernel.py           |  109 +-
 parcels/interaction/neighborsearch/base.py         |   50 +-
 parcels/interaction/neighborsearch/basehash.py     |   21 +-
 parcels/interaction/neighborsearch/hashflat.py     |   23 +-
 .../interaction/neighborsearch/hashspherical.py    |   73 +-
 parcels/kernel.py                                  |  328 +++-
 parcels/particle.py                                |  118 +-
 parcels/particledata.py                            |  232 ++-
 parcels/particlefile.py                            |  245 ++-
 parcels/particleset.py                             |  599 ++++--
 parcels/rng.py                                     |   31 +-
 parcels/tools/converters.py                        |  115 +-
 parcels/tools/exampledata_utils.py                 |    9 +-
 parcels/tools/global_statics.py                    |    6 +-
 parcels/tools/interpolation_utils.py               |  175 +-
 parcels/tools/statuscodes.py                       |   40 +-
 parcels/tools/timer.py                             |   30 +-
 pyproject.toml                                     |   89 +-
 tests/test_advection.py                            |  566 ++++--
 tests/test_data/create_testfields.py               |  117 +-
 tests/test_diffusion.py                            |  128 +-
 tests/test_fieldset.py                             | 1234 +++++++-----
 tests/test_fieldset_sampling.py                    |  918 ++++++---
 tests/test_grids.py                                |  802 +++++---
 tests/test_interaction.py                          |  232 ++-
 tests/test_kernel_execution.py                     |  208 +-
 tests/test_kernel_language.py                      |  431 ++--
 tests/test_mpirun.py                               |   50 +-
 tests/test_particlefile.py                         |  307 +--
 tests/test_particles.py                            |   91 +-
 tests/test_particlesets.py                         |  374 ++--
 tests/test_tools.py                                |    8 +-
 45 files changed, 8805 insertions(+), 4510 deletions(-)

erikvansebille · 2024-08-13T11:13:10Z

Thanks for looking into this, @VeckoTheGecko. Actually, I'm not against 88 line width, I think. Yes, it will lead to more lines, but especially on browsers it's easier to read. So I think my vote would be for 88?

VeckoTheGecko · 2024-08-13T11:32:44Z

@erikvansebille Hmm, yes I agree with the 88 line width wrt. browsers and the docs. Maybe we can do 88 for everything in the docs folder (notebooks/scripts) and then 120 for the rest? Ruff is quite configurable on a file level so this should be fine.

My main concern is readability, since the diff created by 88 really makes files like field.py unreadable and would require major refactoring.

ll88.patch
ll120.patch

EDIT: fixed the patches

erikvansebille · 2024-08-13T11:35:09Z

Ah interesting; if we can do it on a directory/file-level then let's go with 88 for docs/* and 120 for parcels/*. And maybe we don't need a max linewidth at all for tests/*?

VeckoTheGecko · 2024-08-13T11:54:16Z

I think 88 for docs, and 120 for parcels and tests. We still want line length so that configs like

        data = {
            "U": np.array(U, dtype=np.float32),
            "V": np.array(V, dtype=np.float32),
            "psu_salinity": np.array(psu_salinity, dtype=np.float32),
            "cons_pressure": np.array(cons_pressure, dtype=np.float32),
            "cons_temperature": np.array(cons_temperature, dtype=np.float32),
        }

don't get collapsed to one line.

I'll make it a separate PR since its a huge diff

VeckoTheGecko added help wanted documentation good first issue Good for new parcels developers coding/Python labels Aug 12, 2024

github-project-automation bot added this to Parcels development Aug 12, 2024

github-project-automation bot moved this to Backlog in Parcels development Aug 12, 2024

VeckoTheGecko self-assigned this Aug 13, 2024

VeckoTheGecko mentioned this issue Aug 13, 2024

Enable Ruff formatting #1655

Merged

VeckoTheGecko changed the title ~~Type annotations~~ Parcels typing and Pyright support Aug 16, 2024

VeckoTheGecko changed the title ~~Parcels typing and Pyright support~~ Parcels typing and mypy support Aug 20, 2024

VeckoTheGecko mentioned this issue Aug 29, 2024

Type annotations, mypy config, pydoclint setup, and other changes #1660

Merged

VeckoTheGecko closed this as completed Aug 29, 2024

github-project-automation bot moved this from Backlog to Done in Parcels development Aug 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parcels typing and mypy support #1653

Parcels typing and mypy support #1653

VeckoTheGecko commented Aug 12, 2024 •

edited

Loading

VeckoTheGecko commented Aug 13, 2024

erikvansebille commented Aug 13, 2024

VeckoTheGecko commented Aug 13, 2024 •

edited

Loading

erikvansebille commented Aug 13, 2024

VeckoTheGecko commented Aug 13, 2024 •

edited

Loading

Parcels typing and mypy support #1653

Parcels typing and mypy support #1653

Comments

VeckoTheGecko commented Aug 12, 2024 • edited Loading

VeckoTheGecko commented Aug 13, 2024

Overviews

erikvansebille commented Aug 13, 2024

VeckoTheGecko commented Aug 13, 2024 • edited Loading

erikvansebille commented Aug 13, 2024

VeckoTheGecko commented Aug 13, 2024 • edited Loading

VeckoTheGecko commented Aug 12, 2024 •

edited

Loading

VeckoTheGecko commented Aug 13, 2024 •

edited

Loading

VeckoTheGecko commented Aug 13, 2024 •

edited

Loading