DOC: Numpy docstrings migration #1341

VeckoTheGecko · 2023-03-26T07:16:37Z

This PR focusses on the following features (in order of implementation):

Initial conversion of rst docstrings to numpydoc format (with the help of pyment)
numpydoc docstring validators to ensure docstring quality
- Install numpydoc docstring validators as part of the Sphinx build process
- Ensure docstrings are compliant with chosen validators
- Tying in docstring validation with CI/CD
Add types to docstring parameters (particularly focussing on popular modules).

PR contributes to #1324, where you can read more on this featureset.

Draft PR for visibility (and to trigger RTD builds).

(given the size of these diffs, let me know if you want me to break into multiple PRs)

When using `this tutorial <https://link1.com>`_ and `this tutorial <https://link2.com>`_ in two spots, theres a colision due to creating the "this tutorial" reference. Using a double underscore makes it an anonymous link resolving the issue

Only adding napolean for now to visualise docstrings. Will add numpydocs to validate later one the docstrings are in a better state.

Also remove invalid syntax in field.py

Worked on following modules: parcels.interaction parcels.interaction.neighborsearch parcels.compilation parcels.rng parcels.tools.statuscodes parcels.tools.converters parcels.tools.loggers parcels.plotting parcels.scripts.plottrajectoriesfile parcels.scripts.get_examples

Also convert codecompiler

… format

VeckoTheGecko · 2023-03-26T07:18:21Z

Done with initial conversion of docstrings to numpy format. Now moving onto docstring validation.

RTD handling docs means that old build path wasn't necessary. Updating to _build (scripts are still useful for local debugging)

VeckoTheGecko · 2023-03-27T16:33:59Z

Numpydoc validators have been included in the build process (basically linting through docstrings, looking for things like "is the argument in the docstring the same as the one in the function call", ensuring the docstring is rendered correctly).

The full list of possible validation checks are outlined in the error codes. Some of the codes are incompatible with the codebase (either outright, or due to the state of many docstrings). Implementing all these codes is a very very low priority, but the small subset of codes at the moment are a good starting point (and additional codes can be included easily down the line if desired).

VeckoTheGecko · 2023-03-27T16:40:00Z

Just confirming @erikvansebille , is this option checked in the Advanced Settings section of the RTD parcels project panel?

erikvansebille · 2023-03-27T17:13:46Z

Just confirming @erikvansebille , is this option checked in the Advanced Settings section of the RTD parcels project panel?

Yep, that checkbox is on (see below), but the default branch is not set. Should I do that?

VeckoTheGecko · 2023-03-28T00:09:14Z

Following on from this (10 year old) conversation, it doesn't look like builds are triggered from PRs originating from branches on forks. No matter though. My RTD build for this branch is available here. We'll review properly when merging new_docs -> master.

the default branch is not set. Should I do that?

No need 😄

These methods do not take a `field` parameter.

add_constant_field does not take parameter "units"

erikvansebille · 2023-03-28T05:36:37Z

Thanks for digging into this

My RTD build for this branch is available here. We'll review properly when merging new_docs -> master.

That link seems to point to the RTD Issue? Can you update to the correct link?

VeckoTheGecko · 2023-03-28T05:42:28Z

Oops, yes, just updated

VeckoTheGecko · 2023-03-28T06:45:35Z

Used the regex \s:$ to search across the codebase for parameters with missing types. Added many types, will leave the rest to be gradually done over a longer timeframe.

PR ready for feedback/merge 😄 (or we can review it all at once when we merge new_docs to master)

erikvansebille · 2023-03-30T19:48:33Z

parcels/fieldset.py

+            +-----------------------------+-----------------------------+-----------------------------+
+            |                             |         V[k,j+1,i+1]        |                             |
+            +-----------------------------+-----------------------------+-----------------------------+
+            |U[k,j+1,i]                   |W[k:k+2,j+1,i+1],T[k,j+1,i+1]|U[k,j+1,i+1]                 |
+            +-----------------------------+-----------------------------+-----------------------------+
+            |                             |         V[k,j,i+1]          +                             |
+            +-----------------------------+-----------------------------+-----------------------------+


These tables don't show as nice as they do in the old version? Compare https://veckoparcels.readthedocs.io/en/docstrings/#parcels.fieldset.FieldSet.from_c_grid_dataset to https://parcels.readthedocs.io/en/latest/#parcels.fieldset.FieldSet.from_c_grid_dataset

erikvansebille · 2023-03-30T19:55:33Z

Great work, @VeckoTheGecko; this new doc-formatting is much more pleasing to the eye indeed. We're not nearly at the same level of bumpy, but at least we're moving in the right direction. ;-)

A few outstanding general points, before we merge:

The title in your https://veckoparcels.readthedocs.io/en/docstrings/ version mentions v2.3.0; will that be the latest after we merge?
The copyright statement on the page bottom could/should be updated to 2023?
Should we add these commits (especially the really large ones) to the .git-blame-ignore-revs file too?

VeckoTheGecko · 2023-04-02T12:23:46Z

The title in your https://veckoparcels.readthedocs.io/en/docstrings/ version mentions v2.3.0; will that be the latest after we merge?

As of 398ba3b the version number is determined automatically using the parcels.__version__ attribute. new_docs (the target of this PR) is at v2.3.0. When we re-sync and merge new_docs into master it will be at the intended version.

The copyright statement on the page bottom could/should be updated to 2023?

Can do. Should we dynamically determine the year for the copyright statement? i.e. update the copyright statement from:

copyright = '2022, The OceanParcels Team'

to

copyright = f"{datetime.datetime.now().year}, The OceanParcels Team"

Happy to fix this in a separate PR.

Should we add these commits (especially the really large ones) to the .git-blame-ignore-revs file too?

I don't think so. The purpose of .git-blame-ignore-revs is to ignore (large) superfluous changes that in the end have no effect on the function of the code (eg. introducing black, introducing isort, whitespace. These are mainly created by automated formatting tools) and would otherwise soil the blame. These commits here mark a shift in how the codebase works with docstrings, and introduce additional edits to the docstrings, which are worth keeping in blame.

erikvansebille · 2023-04-02T13:05:54Z

OK is fine, let's then just go for it. The integration tests have skipped (not sure why), but I guess we can merge without these in this case, because only docstrings are affected in this PR?

VeckoTheGecko · 2023-04-02T14:11:03Z

The integration tests have skipped (not sure why)

#1335 rearing its head again 😕

And yes, only docstrings/doc related changes.

VeckoTheGecko added 12 commits March 20, 2023 15:40

Refactor docstring validation to use pydocstyle

4b0b391

Some misc. docstring linting fixes

2490c8c

Convert field.py to use numpydoc

2ca6af0

Preliminart numpydoc sphinx support

a40d584

Only adding napolean for now to visualise docstrings. Will add numpydocs to validate later one the docstrings are in a better state.

Convert fieldset.py to use numpydoc

7c15b53

Also remove invalid syntax in field.py

Convert kernel/*.py files to use numpydoc

eade4d4

Also convert codecompiler

Convert particleset/*.py files to use numpydoc

0dc5da7

Convert grid.py, particle.py, particlefile/*.py files to use numpydoc

589f066

Convert application_kernels/*.py files to use numpydoc

b123e6a

Final conversions and touchups of .py files to use numpydoc docstring…

fd47448

… format

VeckoTheGecko added 3 commits March 26, 2023 22:35

Patch existing sphinx errors/warnings

f05e303

Update docs build scripts build location

425d648

RTD handling docs means that old build path wasn't necessary. Updating to _build (scripts are still useful for local debugging)

Numpydoc support for docstring validation

93a8a77

VeckoTheGecko added 2 commits March 28, 2023 12:23

Fix density docstrings

da542d8

These methods do not take a `field` parameter.

Fix add_constant_field docstring

977e24d

add_constant_field does not take parameter "units"

VeckoTheGecko added 2 commits March 28, 2023 14:30

Adding parameter types and clarifying some documentation

602b4f2

More types (and boolean cleanup)

f5fdd79

VeckoTheGecko requested a review from erikvansebille March 28, 2023 06:45

VeckoTheGecko marked this pull request as ready for review March 28, 2023 06:47

erikvansebille reviewed Mar 30, 2023

View reviewed changes

VeckoTheGecko merged commit 6f02481 into OceanParcels:new_docs Apr 2, 2023
6 of 7 checks passed

VeckoTheGecko deleted the docstrings branch April 2, 2023 14:15

VeckoTheGecko mentioned this pull request Apr 3, 2023

DOC: API reference restructure, contributing guidelines, and more... #1342

Merged

9 tasks

VeckoTheGecko mentioned this pull request Apr 10, 2023

Documentation revamp #1346

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC: Numpy docstrings migration #1341

DOC: Numpy docstrings migration #1341

VeckoTheGecko commented Mar 26, 2023 •

edited

VeckoTheGecko commented Mar 26, 2023

VeckoTheGecko commented Mar 27, 2023

VeckoTheGecko commented Mar 27, 2023

erikvansebille commented Mar 27, 2023

VeckoTheGecko commented Mar 28, 2023 •

edited

erikvansebille commented Mar 28, 2023

VeckoTheGecko commented Mar 28, 2023 •

edited

VeckoTheGecko commented Mar 28, 2023 •

edited

erikvansebille Mar 30, 2023

erikvansebille commented Mar 30, 2023

VeckoTheGecko commented Apr 2, 2023 •

edited

erikvansebille commented Apr 2, 2023

VeckoTheGecko commented Apr 2, 2023

DOC: Numpy docstrings migration #1341

DOC: Numpy docstrings migration #1341

Conversation

VeckoTheGecko commented Mar 26, 2023 • edited

VeckoTheGecko commented Mar 26, 2023

VeckoTheGecko commented Mar 27, 2023

VeckoTheGecko commented Mar 27, 2023

erikvansebille commented Mar 27, 2023

VeckoTheGecko commented Mar 28, 2023 • edited

erikvansebille commented Mar 28, 2023

VeckoTheGecko commented Mar 28, 2023 • edited

VeckoTheGecko commented Mar 28, 2023 • edited

erikvansebille Mar 30, 2023

Choose a reason for hiding this comment

erikvansebille commented Mar 30, 2023

VeckoTheGecko commented Apr 2, 2023 • edited

erikvansebille commented Apr 2, 2023

VeckoTheGecko commented Apr 2, 2023

VeckoTheGecko commented Mar 26, 2023 •

edited

VeckoTheGecko commented Mar 28, 2023 •

edited

VeckoTheGecko commented Mar 28, 2023 •

edited

VeckoTheGecko commented Mar 28, 2023 •

edited

VeckoTheGecko commented Apr 2, 2023 •

edited