Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove recipe filler utility #3777

Merged
merged 3 commits into from
Nov 13, 2024
Merged

Remove recipe filler utility #3777

merged 3 commits into from
Nov 13, 2024

Conversation

schlunma
Copy link
Contributor

@schlunma schlunma commented Oct 15, 2024 •
edited
Loading

Description

With the introduction of the wildcard syntax for recipes in ESMValGroup/ESMValCore#1609 the recipe filler utility has become obsolete. After parsing a recipe, ESMValTool will automatically create a "filled" version of the recipe (available in the run output directory) with all wildcards expanded, which can then be modified by the user and run again.

Details on how to use the new wildcard syntax: https://docs.esmvaltool.org/projects/ESMValCore/en/latest/recipe/overview.html#automatically-populating-a-recipe-with-all-available-datasets

Example notebook: https://github.com/ESMValGroup/ESMValCore/blob/main/notebooks/discovering-data.ipynb

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:

@schlunma schlunma added this to the v2.12.0 milestone Oct 15, 2024
@schlunma schlunma self-assigned this Oct 15, 2024
@schlunma schlunma mentioned this pull request Oct 15, 2024
Merged
8 tasks
valeriupredoi
valeriupredoi requested changes Oct 15, 2024
View reviewed changes
Copy link
Contributor

@valeriupredoi valeriupredoi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, Manu, been tied up all day today fixing all manners of technical gubbins in ESMValTool. Let me have a closer look at this tomorrow - have you asked around about this, I know there are quite a few folks that still use the recipe filler eg @ledm @egalytska etc - note that the wildcard approach is not exactly equivalent with the recipe filler: that will find and run everything that is available, whereas the recipe filler will tell the user what is available before the actual run, then the user will decide what data to run with

@schlunma
Copy link
Contributor Author

Don't worry, I would never merge this without proper discussion! Sorry, opening this PR takes as much time as opening an issue, so I thought I might as well do that!

Regarding the features:

note that the wildcard approach is not exactly equivalent with the recipe filler: that will find and run everything that is available, whereas the recipe filler will tell the user what is available before the actual run, then the user will decide what data to run with

This can also be achieved with the wildcard syntax. For every ESMValTool run, a filled recipe is written which, in case wildcards are used, contains all expanded datasets. User can modify this as they wish to achieve the same functionality as the recipe filler.

Also, the wildcard syntax fixes some of the limitations of the recipe filler:

the tool doesn’t yet work with derived variables; it will not return any available datasets; operation restricted to CMIP data only, OBS lookup is not available yet.

@valeriupredoi
Copy link
Contributor

Oh that's very nice about the filled recipe! Didn't know about that feature - that's that sold to me, in this case, but let's still ask some of the users, not being a user per se myself, am not aware of potential used cases users may be employing the recipe filler for. Thanks, Manu 🍺

@egalytska
Copy link
Contributor

That all sounds good to me.
However, there is one minor drawback in my case. In the paper I wrote that I used recipe_filler to get the info regarding available variables from the models. If someone by any miracle decides to search for this recipe filler, they obviously won't find it. This can be quite annoying. As a workaround, would it be possible to mention that recipe_filler is deprecated and wildcard syntax for recipes has the same (or improved) functionality?
Best regards to everyone 💖

@valeriupredoi
Copy link
Contributor

@egalytska indeed we'll mention that as you say, in the 2.12 release notes, and the recipe filler still lives in history ie in 2.11 and before 🍺

@schlunma
Copy link
Contributor Author

@ESMValGroup/esmvaltool-developmentteam In this pull request we plan to remove the recipe filler utility because it has become obsolete (see details on the pull request description above). If you are a regular user of this tool and/or think we should not remove it, please let us know as soon as possible by commenting here. Thanks!!

@valeriupredoi

This comment was marked as off-topic.

Sign in to view
@esmvalbot

This comment was marked as off-topic.

Sign in to view
@esmvalbot

This comment was marked as off-topic.

Sign in to view
@schlunma
Copy link
Contributor Author

It looks like no one disagrees with this, so I guess we can safely go ahead with this PR! @valeriupredoi if you agree, would you please merge this? Thanks! 🍻

valeriupredoi
valeriupredoi approved these changes Nov 13, 2024
View reviewed changes
Copy link
Contributor

@valeriupredoi valeriupredoi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RIP, Recipe Filler, you was a gud boi 😭 Cheers, Manu 🍺

@valeriupredoi
Copy link
Contributor

devel tests are with iris=3.11 and those fails are fixed by #3810

@valeriupredoi valeriupredoi merged commit eb62759 into main Nov 13, 2024
7 of 9 checks passed
@valeriupredoi valeriupredoi deleted the remove_recipe_filler branch November 13, 2024 12:16
ehogan added a commit that referenced this pull request Nov 19, 2024
@ehogan
Merge branch 'main' into recipe_test_workflow_prototype
09490de 
* main: (31 commits)
  Update environment: pin `iris>=3.11`, unpin `cartopy` and allow for `numpy >=2` (#3811)
  Fix issue related to removal/change of private function imported in `diag_scripts/shared/_supermeans.py` (deprecation in iris=3.11) (#3810)
  Remove recipe filler utility (#3777)
  [Condalock] Update Linux condalock file (#3809)
  change authors name (#3806)
  [Condalock] Update Linux condalock file (#3798)
  Fix contourf plots for masked data (#3797)
  [Condalock] Update Linux condalock file (#3796)
  Add next release schedule (#3794)
  Use `transform_first=True` for contourf plots with Robinson projection to avoid cartopy bug (#3789)
  Pin pys2index >=0.1.5 in osx environment (#3792)
  Adding a CMORiser for CMAP data for pr (#3766)
  Adding pr, tauu, tauv, tos to NCEP2 CMORISer (#3765)
  Readthedocs configuration/builds: revert to miniconda before miniforge is available (#3785)
  Adapt ESMValTool to new configuration (#3761)
  Adding pr, tauu, tauv NOAA-CIRES-20CR-V2 CMORISER (#3763)
  update comment in conda lock creation Github action (#3788)
  [Condalock] Update Linux condalock file (#3786)
  update Docker builds badge in README (#3783)
  Pin mamba<2 for conda-lock: solution by Ben Mares @maresb (#3771)
  ...
ehogan added a commit that referenced this pull request Nov 19, 2024
@ehogan
Merge branch '3723_remove_rose_cylc' of github.com:ESMValGroup/ESMVal…
90c8ca1 
…Tool into 3723_remove_rose_cylc

* '3723_remove_rose_cylc' of github.com:ESMValGroup/ESMValTool: (31 commits)
  Update environment: pin `iris>=3.11`, unpin `cartopy` and allow for `numpy >=2` (#3811)
  Fix issue related to removal/change of private function imported in `diag_scripts/shared/_supermeans.py` (deprecation in iris=3.11) (#3810)
  Remove recipe filler utility (#3777)
  [Condalock] Update Linux condalock file (#3809)
  change authors name (#3806)
  [Condalock] Update Linux condalock file (#3798)
  Fix contourf plots for masked data (#3797)
  [Condalock] Update Linux condalock file (#3796)
  Add next release schedule (#3794)
  Use `transform_first=True` for contourf plots with Robinson projection to avoid cartopy bug (#3789)
  Pin pys2index >=0.1.5 in osx environment (#3792)
  Adding a CMORiser for CMAP data for pr (#3766)
  Adding pr, tauu, tauv, tos to NCEP2 CMORISer (#3765)
  Readthedocs configuration/builds: revert to miniconda before miniforge is available (#3785)
  Adapt ESMValTool to new configuration (#3761)
  Adding pr, tauu, tauv NOAA-CIRES-20CR-V2 CMORISER (#3763)
  update comment in conda lock creation Github action (#3788)
  [Condalock] Update Linux condalock file (#3786)
  update Docker builds badge in README (#3783)
  Pin mamba<2 for conda-lock: solution by Ben Mares @maresb (#3771)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@valeriupredoi valeriupredoi valeriupredoi approved these changes

@bouweandela bouweandela Awaiting requested review from bouweandela

Assignees

@schlunma schlunma

Labels
backwards incompatible change
Projects
None yet
Milestone
v2.12.0
Development

Successfully merging this pull request may close these issues.
3 participants
@schlunma @valeriupredoi @egalytska