diff --git a/doc/source/Code-Style-Guide.rst b/doc/source/Code-Style-Guide.rst index 11a8c89566..c990797ede 100644 --- a/doc/source/Code-Style-Guide.rst +++ b/doc/source/Code-Style-Guide.rst @@ -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 ~~~~~~~~~~~~~ @@ -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 `_ +e.g. `Threshold `_ class. In some limited cases an iris ``CubeList`` may be preferable. Avoid writing code that can do both. Class names use `PascalCase `_ whilst @@ -564,7 +566,7 @@ Add a command line interface (improver/cli/.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 `` (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 @@ -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 @@ -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 ``. 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. diff --git a/doc/source/Dependencies.rst b/doc/source/Dependencies.rst index 8044676081..148c917e6d 100644 --- a/doc/source/Dependencies.rst +++ b/doc/source/Dependencies.rst @@ -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 diff --git a/doc/source/Guidance-for-Reviewing-Code.rst b/doc/source/Guidance-for-Reviewing-Code.rst index 8e9198fbf1..2faf7ab0ec 100644 --- a/doc/source/Guidance-for-Reviewing-Code.rst +++ b/doc/source/Guidance-for-Reviewing-Code.rst @@ -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. @@ -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** @@ -78,8 +73,10 @@ 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** @@ -87,24 +84,11 @@ As a Reviewer: * 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. diff --git a/doc/source/How-to-implement-a-command-line-utility.rst b/doc/source/How-to-implement-a-command-line-utility.rst index cbba757144..a13aa7a35c 100644 --- a/doc/source/How-to-implement-a-command-line-utility.rst +++ b/doc/source/How-to-implement-a-command-line-utility.rst @@ -76,13 +76,9 @@ checksums listed in `_. 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 `_. +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 ~~~~~~~~~~~~~~~~ diff --git a/doc/source/Running-at-your-site.rst b/doc/source/Running-at-your-site.rst index ac7cea27a1..2e4fdc7693 100644 --- a/doc/source/Running-at-your-site.rst +++ b/doc/source/Running-at-your-site.rst @@ -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 `_ 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 `_. The -`getting started Jupyter -notebook `_ -in that repository also provides examples of the data structure. - Test suite ---------- @@ -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 `_ 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 diff --git a/doc/source/about.rst b/doc/source/about.rst index 92879e3bd0..02af746e2e 100644 --- a/doc/source/about.rst +++ b/doc/source/about.rst @@ -63,9 +63,9 @@ IMPROVER can be installed using pip or conda with installation instructions avai git clone https://github.com/metoppv/improver.git -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 ==================== @@ -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