Parcels typing and mypy support

VeckoTheGecko commented 2 months ago

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 #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

VeckoTheGecko commented 2 months ago

@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"

88 line length "git diff --stat"

erikvansebille commented 2 months ago

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

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

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

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

OceanParcels / Parcels

Parcels typing and mypy support #1653

Overviews