Skip to content

Commit

Permalink
Updated Readthedocs documentation in line with current practices. (#2048
Browse files Browse the repository at this point in the history
)

* Updated Readthedocs documentation in line with current practices.

* Reverting changes to this file

* Responding to review comments.

* Updates as per review

* Updated target for environment_a link.

---------

Co-authored-by: Katharine Hurst <[email protected]>
  • Loading branch information
Kat-90 and Katharine Hurst authored Nov 21, 2024
1 parent 245cf18 commit 6e4a898
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 71 deletions.
13 changes: 10 additions & 3 deletions doc/source/Code-Style-Guide.rst
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ General comments
line interfaces) have acceptance tests.
* Use type annotations on function and method interfaces, except for
CLIs.
* Avoid adding functionality to CLIs beyond calling the relevant plugin,
all other functionality should be added to the plugin itself.

Pull requests
~~~~~~~~~~~~~
Expand Down Expand Up @@ -387,7 +389,7 @@ Plugins (classes) should be an example of a non-trivial algorithm or set
of algorithms for a particular purpose. They should be set up via the
``__init__`` method and then invoked on a particular iris Cube ``cube``
using a ``process`` method - e.g. using ``process(cube)``. See
e.g. `Threshold <https://github.com/metoppv/improver/blob/master/lib/improver/threshold.py>`_
e.g. `Threshold <https://github.com/metoppv/improver/blob/master/improver/threshold.py>`_
class. In some limited cases an iris ``CubeList`` may be preferable.
Avoid writing code that can do both. Class names use
`PascalCase <https://en.wikipedia.org/wiki/PascalCase>`_ whilst
Expand Down Expand Up @@ -564,7 +566,7 @@ Add a command line interface (improver/cli/<cli_name>.py) to invoke plugins
that can be used as a standalone utility or executable within a suite context
(e.g. wind downscaling, neighbourhood processing, spot data extraction).
These CLIs are invoked using ``bin/improver <cli-name>`` (note that the
CLI filename uses underscores, but the call to use the CLI uses hyphens)
CLI filename uses underscores, but the call to use the CLI uses hyphens).

IMPROVER CLIs should only have ``from improver import cli`` as the top
level imports. Other imports are placed inside the function that uses
Expand All @@ -579,6 +581,10 @@ to save a cube to disk, it will need the decorator ``@cli.with_output``,
this will mean on the command line, the ``--output`` flag can be used to
specify an output path.

As mentioned above, it is important to ensure that no functionality
other than calling the plugin exists within the CLI layer.
Any checks on the data or input requirements should be done in the plugin itself.

To load the cubes, each cube argument will need a type. For a basic cube
this will be ``cube: cli.inputcube``. If there is a default argument to
go with the typed variable, spaces are required around the ``=`` for
Expand Down Expand Up @@ -810,5 +816,6 @@ New release steps:
case just check it. The checksum of the compressed ``.tar.gz`` IMPROVER
source code can be obtained via ``openssl sha256 <file name>``.
Currently the people with write access to the improver-feedstock
repository are @benfitzpatrick, @PaulAbernethy, @tjtg and @lucyleeow.
repository are @benfitzpatrick, @PaulAbernethy, @tjtg, @cpelley and
@dementipl.
You can ping one of these people to merge your pull request.
2 changes: 1 addition & 1 deletion doc/source/Dependencies.rst
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ cartopy
~~~~~~~~~~~~~~~~~
Cartopy is used for grid projections and coordinate transformations.

https://scitools.org.uk/cartopy/docs/stable/
https://scitools.org.uk/cartopy/docs/latest/


cftime
Expand Down
50 changes: 17 additions & 33 deletions doc/source/Guidance-for-Reviewing-Code.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@ As a Developer:
branch up to origin.
2. Check the code with respect to the standards in the Definition of
Done. Also check that new unit tests do not raise any warnings.
3. Create a pull request. A main reviewer must be assigned, but adding
multiple reviewers to the pull request is encouraged to help share
expertise. Please ask for help assigning reviewers if you need it.
3. Create a pull request and notify a member of the team so it can be
assigned to an appropriate reviewer.
4. The relevant issue should be referenced within the pull request.
5. Respond to the reviewers’ comments and update the pull request
accordingly. This will be an iterative process.
Expand All @@ -26,35 +25,31 @@ As a Developer:
perspective, ensure that a subject matter expert is satisfied with
the changes.
10. (Internal staff only:) Prior to moving the issue into the 'Done'
column, notify the Product Owner that the issue has been completed,
and discuss the functionality implemented.
column, assign the ticket back to the original developer and notify
them in preparation for merging into the codebase.

As a Reviewer:
--------------

1. Issues should be assigned to a main reviewer. Adding multiple
reviewers to the pull request is encouraged to help share expertise.
Please ask for help assigning reviewers if you need it.
1. All reviewers are encouraged to add comments to the pull request.

2. All reviewers are encouraged to add comments to the pull request.

3. Main reviewers should:
2. Reviewers should:

a. **Read the code and post in-line comments for suggested
alterations.**
b. **Ensure unit tests are run and pass.**
c. **Ensure command line interface acceptance tests run and pass.
These must be ran on the desktop using bin/improver tests as they
are not run by travis**
These must be run on the desktop using bin/improver tests,
see :doc:`Running-at-your-site` for more information**
d. **The Acceptance Criteria defined within the associated issue has
been met.**
e. **The criteria within the Definition of Done has been satisfied.**
f. **Ensure their testing is documented on the issue.**

4. Main reviewers should post comments to the pull request to show that
3. Reviewers should post comments to the pull request to show that
they have completed: a, b, c, d, e, f.

5. Things to consider when reading through the code are:
4. Things to consider when reading through the code are:

a. **Naming conventions and coding style**

Expand All @@ -78,33 +73,22 @@ As a Reviewer:

d. **Functionality**

* Does the code do what is supposed to?
* Does the code do what it's supposed to?
* Are errors handled appropriately?
* Is the code efficient with respect to processing time and
memory requirements?

e. **Test coverage**

* Do the tests provided cover the expected situations?
* Are the tests understandable?
* Have edge cases been considered?

6. If this is a first review, the developer should then move the issue
5. If this is a first review, the developer should then move the issue
into 'Second Review' and a second reviewer should ensure that the
criteria listed above have been met. The Scrum Master can be
consulted, if necessary.

7. If this is a second review, the developer should contact the Product
Owner prior to moving the issue into the 'Done' column. The Scrum
Master can be consulted, if necessary.

Before merging:
---------------

Always rerun Travis before merging a PR. Do this by:

1. At the bottom of the PR, click ``show all checks`` and then ``details`` next
to the Travis check.
2. Click ``Restart build``

If you haven’t already signed into Travis with GitHub it will prompt you
to do this the first time you try this, and it may take a few minutes to
sync before you can see the page with the option to restart the build.
6. If this is a second review, the developer should assign the issue back
to the developer and contact them prior to moving the issue into the
'Done' column. The Scrum Master can be consulted during the Stand-up.
10 changes: 3 additions & 7 deletions doc/source/How-to-implement-a-command-line-utility.rst
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,9 @@ checksums listed in
<https://github.com/metoppv/improver/blob/master/improver_tests/acceptance/SHA256SUMS>`_.
Changes to the checksum file in the code repository identify when
changes are made to the code requiring corresponding changes to the data
files. The acceptance test data files are maintained internally in a Met
Office repository, with the directory structure being based on the CLI
plugin names.

Minimising the usage of input and output data files is desirable in the
future, as described in issue
`#1218 <https://github.com/metoppv/improver/issues/1218>`_.
files. The acceptance test data files are maintained in a public repository:
`https://github.com/metoppv/improver_test_data`, with the directory
structure being based on the CLI plugin names.

Use of checksums
~~~~~~~~~~~~~~~~
Expand Down
39 changes: 15 additions & 24 deletions doc/source/Running-at-your-site.rst
Original file line number Diff line number Diff line change
Expand Up @@ -99,24 +99,6 @@ At the Python interpreter prompt:
print(output)
iris.save(output, "output.nc")
Input data
----------

IMPROVER processes standardised data in NetCDF format.

The main Met Office weather models have data available on `Amazon Web
Services Open Data
registry <https://registry.opendata.aws/uk-met-office/>`_ which is
Creative Commons BY-NC-ND licenced (free to use for non-commercial
purposes) and compatible with IMPROVER.

There are some examples of how to retrieve and use the data on the `Met
Office aws-earth-examples Github
repository <https://github.com/MetOffice/aws-earth-examples>`_. The
`getting started Jupyter
notebook <https://github.com/MetOffice/aws-earth-examples/blob/master/examples/1.%20Getting%20Started.ipynb>`_
in that repository also provides examples of the data structure.

Test suite
----------

Expand All @@ -131,24 +113,33 @@ tests are quick to run. Unit tests are run as part of the test suite on
# Run unit tests via improver-tests wrapper
bin/improver-tests unit
bin/improver-tests --help # Prints out the help information
# Use pytest directly with marker to run only the unit tests
pytest -m 'not acc'
# To run a particular function within a unit test, you can use the :: notation
pytest -m improver_tests/test_unit_test.py::Test_function
The CLI (command line interface) acceptance tests use known good output
(KGO) files on disk for validating that the behaviour is as expected.
These data files are large, so the acceptance tests are not run on
Github actions. Contact a `Met Office IMPROVER
contributor <https://github.com/metoppv/improver/commits/master>`_ to
arrange for a copy of the acceptance test input and output files.
(KGO) files for validating that the behaviour is as expected. This data
can be found in the `improver_test_data` open source repository on GitHub.

The path to the acceptance test data is set using the
``IMPROVER_ACC_TEST_DIR`` environment variable. Acceptance tests will be
skipped if this environment variable is not defined.
To run the acceptance tests you can use the following:

.. code:: bash
export IMPROVER_ACC_TEST_DIR=/path/to/acceptance/data
export IMPROVER_ACC_TEST_DIR=/path/to/acceptance/data/repo
# Use pytest marker to run only the acceptance tests
pytest -m acc
# Acceptance tests can be run significantly faster in parallel using the pytest-xdist plugin
pytest -n 8
# An example of running just one particular acceptance test
pytest -v -s -m acc -k test_cli_name.py
To run all tests together at once, the following command can be input

.. code:: bash
bin/improver-tests # runs all tests
6 changes: 3 additions & 3 deletions doc/source/about.rst
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ IMPROVER can be installed using pip or conda with installation instructions avai
git clone https://github.com/metoppv/improver.git <local directory>
The list of dependencies can be found in the `environment.yml`_ file.
The list of dependencies can be found in the `environment_a.yml`_ file.

.. _environment.yml: https://github.com/metoppv/improver/blob/master/environment.yml
.. _environment_a.yml: https://github.com/metoppv/improver/blob/master/envs/environment_a.yml

Example use of a CLI
====================
Expand Down Expand Up @@ -126,7 +126,7 @@ When citing IMPROVER, we recommend using the sources in the first two rows of th

.. _Roberts et al., 2023: https://doi.org/10.1175/BAMS-D-21-0273.1
.. _IMPROVER - The New Probabilistic Postprocessing System at the Met Office (BAMS 2023): https://doi.org/10.1175/BAMS-D-21-0273.1
.. _Archive of latest released version of IMPROVER (Zenodo): https://doi.org/10.5281/zenodo.8410114
.. _Archive of latest released version of IMPROVER (Zenodo): https://zenodo.org/records/13354071
.. _A post-processing and verification strategy for the future (MOSAC 2015): https://github.com/metoppv/improver/tree/master/doc/files/MOSAC_2015_20.19_Post-processing-verification.pdf
.. _IMPROVER - the new post processing and verification system (MOSAC 2019): https://github.com/metoppv/improver/tree/master/doc/files/MOSAC_23.9_Roberts_Paper_171218.pdf
.. _Generating probabilistic forecasts from convection permitting ensembles: https://presentations.copernicus.org/EMS2017-277_presentation.pdf
Expand Down

0 comments on commit 6e4a898

Please sign in to comment.