Detailed Working Practices

Overview

• Create an issue
• Create a new feature branch from main
• Activate the development environment
• Make changes
  • Modify the code
  • Write tests and documentation
  • Validate the workflow locally
  • Run the tests locally
  • Build the documentation locally
  • Commit the code changes
• Create a pull request
• Review the code changes
• Merge the feature branch into main
• Remove the feature branch
• Close the issue

Create an issue

• If an issue has not yet been created, create a New Issue by selecting the appropriate template, then:
  • on the right hand side of the page, click on the cog symbol to the right of:
    • Assignees to add yourself as an assignee
    • Labels to add appropriate labels
    • Projects to add the issue to the Climate Model Evaluation Workflow (CMEW) project board
    • Milestone to add the appropriate milestone
  • do not add a Type (the decision was made not to use Type, since it is configured at the organisational level and cannot be configured at the repository level)

Create a new feature branch from main

• Checkout and update the main branch:

  cd <desired_repository_location>/CMEW
  git checkout main
  git pull
  

• Create and checkout a new feature branch from the main branch, where <feature_branch_name> is <issue_number>_<short_description_of_feature>, e.g. 5_develop_configure_task:

  git checkout -b <feature_branch_name>
  

• Share the feature branch to GitHub:

  git push --set-upstream origin <feature_branch_name>
  

Activate the development environment

• Activate the environment for the current session, where <version> is the version of ESMValTool to use (the following commands only need to be executed once per session):

  module load scitools/community/esmvaltool/<version>
  export CYLC_VERSION=8
  

Make changes

Modify the code

• Make changes to address the issue

Coding requirements

• All files in the CMEW repository must start with the following two comments, where <year> is either <year_of_first_publication_of_the_file> or <year_of_first_publication_of_the_file>-<year_of_last_modification_of_the_file>:
  • Python (.py), text files (.txt), TOML (.toml), YAML (.yaml or .yml):

    # (C) Crown Copyright <year>, Met Office.
    # The LICENSE.md file contains full licensing details.
    

  • Markdown (.md):

    [(C) Crown Copyright <year>, Met Office.]: #
    [The LICENSE.md file contains full licensing details.]: #
    

  • reStructuredText (.rst):

    .. (C) Crown Copyright <year>, Met Office.
    .. The LICENSE.md file contains full licensing details.
    

• The Cylc: Workflow Design Guide must be followed to ensure CMEW is clear, maintainable, and portable.
• Use f-strings in Python code.
• Do not use type hints.

Write tests and documentation

Testing requirements

Workflow tests

• Details will need to be added here about adding new workflow tests, should they be needed (at the moment, the workflow tests consist of running the radiation_budget assessment area (which takes a few minutes) and a compare task, which confirms that the expected outputs have been produced).

Python tests

• pytest is used to run the Python tests in CMEW.
• Python test modules must be located in the app/unittest/tests/ directory, in a parallel directory structure to the Python modules they are testing, e.g. tests for the app/configure_standardise/bin/create_variables_file.py module must be located in the app/unittest/tests/test_configure_standardise/test_create_variables_file.py test module.
• Test function names must have the form test_<name_of_thing_being_tested>_<additional_description>, where <additional_description> uses full words to describe the specific feature of <name_of_thing_being_tested>.
• Test functions should typically test one thing (often this means one assert statement, but not always) and follow the pattern:

  def test_my_function_for_reason():
      actual = my_function(arg1, arg2, ...)
      expected = <expected_output>
      assert actual == expected
  

• Use np.testing.assert_allclose when comparing arrays / array-like objects.
• Test functions should not have docstrings (the tests are excluded from the Sphinx documentation build); instead, use comments to document the tests.

Documentation requirements

General documentation

• Use inclusive language that doesn't refer to "seeing" an element, for example "XYZ provides more information".
• Capitalised acronyms should be used when using acronyms.
• Limit all lines in markdown (.md) files and .rst files to a maximum of 79 characters (Markdown documentation provides exceptions to this rule).
• However, do not break the content across multiple lines at 79 characters, but rather break them on semantic meaning (e.g. periods or commas). Brandon Rhodes: Semantic Linefeeds provides more information.
• Use comments appropriately.

Workflow documentation

• All environmental variables defined in the rose-suite.conf file must have a corresponding entry (in alphabetical order) in the meta/rose-meta.conf file.

Markdown documentation

• Use reference style links to help limit all lines in markdown (.md) files to a maximum of 79 characters (Note: it is ok for the URL in this case to be more than 79 characters).

Python documentation

• Use the ​NumPy Docstring Standard when writing docstrings for Python code in CMEW.
• All parameters must be described in the docstrings, including their types.
• All docstrings must contain a summary line that starts with Return <description>.. The 4th point in the Notes section in PEP 257: Docstring Conventions provides more information.
• The line lengths of all docstrings and comments should be limited to 72 characters. PEP 8: Style Guide for Python Code provides more information.
• All sentences in docstrings and comments must start with an upper case letter, end in a period and use the correct grammar; all words must be spelt correctly.

Validate the workflow locally

• Navigate to the workflow directory:

  cd <desired_repository_location>/CMEW/CMEW
  

• Validate the Cylc workflow:

  cylc validate -O metoffice .
  

  Note: cylc validate only works after pre-processing Jinja2 branches, and when SITE is specified (e.g. -O metoffice).
• Run the Cylc configuration linter to help spot issues in unprocessed files:

  cylc lint
  

• Run all Rose validator macros against the Rose configuration files:

  rose macro -V
  

• Validate the format of Rose configuration files, and commit any changes:

  rose config-dump
  

• Validate the Rose configuration metadata:

  rose metadata-check -C meta/
  

• For information: the GitHub Workflow also automatically validates the workflow every time a commit is pushed to GitHub.

Run the tests locally

• Navigate to the workflow directory:

  cd <desired_repository_location>/CMEW/CMEW
  

• Run the Python unit tests and the workflow end-to-end tests; the workflow end-to-end tests ensure that the expected output files are generated by CMEW (ESMValTool tests, e.g. the Recipe Test Workflow, ensures the contents of the output files are as expected):

  cylc vip -O metoffice -O test
  

• Ensure the tests pass. One way to do this is to check Cylc Review.

Build the documentation locally

• Navigate to the documentation directory:

  cd <desired_repository_location>/CMEW/doc
  

• Build the documentation:

  make html
  

• View the documentation:

  gio open build/html/index.html
  

• For information: the GitHub Workflow also automatically builds the documentation every time a commit is pushed to GitHub.

Commit the code changes

• Commit all local and staged changes:

  git commit -am "#<issue_number>: <descriptive_commit_message>"
  

• If one or more of the pre-commit git hooks fail, the git commit command will fail and local modifications to the code will be made; commit all local and staged changes again:

  git commit -am "#<issue_number>: <descriptive_commit_message>"
  

• Share the changes to GitHub:

  git push
  

Create a pull request

• Create either a PR or, if the changes aren't quite ready for a formal review, a draft PR. GitHub: Creating the pull request provides more information.
• Ensure all the items in the checklist provided on the PR are complete before adding the reviewer(s).
  • Note that adding Closes #<issue_number>. in the first comment box of the PR links the PR to the issue, which enables the issue to be closed automatically when the PR is merged. GitHub: Linking a pull request to an issue provides more information.
• If a draft PR was created, once the changes are ready for review, mark the PR as ready for review. GitHub: Marking a pull request as ready for review provides more information. Then add the reviewer(s).
• Note that the reviewer(s) are only notified that the PR is ready for review when they are added to the PR.

Review the code changes

• Wiki: Review Process provides more information.
• Optionally test the code locally as a reviewer:
  • For first time reviewers, clone the repository by following the first three points in Wiki: First time developer instructions.
  • Checkout and update the main branch by following the first point in Wiki: Create a new feature branch from main.
  • Checkout the feature branch to be reviewed: git checkout <feature_branch_name>.
  • Follow Wiki: Run the tests locally.

Merge the feature branch into main

• Once a Reviewer has approved the PR, the developer must merge the feature branch into main. GitHub: Merging a pull request provides more information:
  • ensure Squash and merge is selected by using the down arrow to the right of the green button.
  • click the green Squash and merge button.
  • move the PR number from the end of the message to the start of the message, e.g. #<PR_number>: <issue_title>.
  • click the green Confirm squash and merge button.
• If the PR is unable to be merged due to merge conflicts, resolve the conflicts, either via the web editor or the command line. GitHub: Resolving a merge conflict on GitHub provides more information:

  git checkout main
  git pull
  git checkout <feature_branch_name>
  git merge main
  <resolve merge conflicts>
  git commit -am "#<issue_number>: Merge latest changes from main"
  git push
  

Remove the feature branch

• Once the PR has been merged, the branch will be automatically deleted from the repository, but will need deleted from your local clone.
• Remove the local branch:

  git branch -d <feature_branch_name>
  

• Remove stale references associated with origin:

  git remote prune origin
  

Close the issue

• If the PR was correctly linked to the issue as described in the GitHub: Create a pull request section, the issue will already be closed.