ESMValGroup / ESMValTool

ESMValTool: A community diagnostic and performance metrics tool for routine evaluation of Earth system models in CMIP
https://www.esmvaltool.org
Apache License 2.0
210 stars 122 forks source link

Add Rose and Cylc to testing dependencies for RTW #3625

Closed ehogan closed 1 month ago

ehogan commented 1 month ago

Description

Before you get started

Checklist

It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the ๐Ÿ›  Technical or ๐Ÿงช Scientific review.


To help with the number of pull requests:

bouweandela commented 1 month ago

Does the workflow actually use the conda environment that cylc is in for running the recipes? I had the impression that it was using containers? If that is the case, there doesn't seem much point in adding cylc et al to the main ESMValTool environment, we could just have a small separate environment with cylc et al for running the recipe test workflow.

valeriupredoi commented 1 month ago

AFAIK only CEDA/JASMIN runs use containers (sifs), but MetO use the buildable environment

ehogan commented 1 month ago

Does the workflow actually use the conda environment that cylc is in for running the recipes? I had the impression that it was using containers? If that is the case, there doesn't seem much point in adding cylc et al to the main ESMValTool environment, we could just have a small separate environment with cylc et al for running the recipe test workflow.

No, as V says we use a container on JASMIN and we use our community (Conda) environment at the MO.

However, I would argue it makes sense for Rose and Cylc to be included in the test dependencies (that was what I was aiming for in the changes, and apologies if I didn't do that correctly!). I imagine a developer might get frustrated if they choose to install the test dependencies, but they don't get an environment that enables them to run the RTW ๐Ÿค”

bouweandela commented 1 month ago

However, I would argue it makes sense for Rose and Cylc to be included in the test dependencies (that was what I was aiming for in the changes, and apologies if I didn't do that correctly!). I imagine a developer might get frustrated if they choose to install the test dependencies, but they don't get an environment that enables them to run the RTW ๐Ÿค”

OK, let's add them then.