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.

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

obs4mips CERES-EBAF: update version to latest available through esgf in recipe_validation.yml #3002

Merged
merged 3 commits into from
Jan 19, 2023

Conversation

valeriupredoi
Copy link
Contributor

@valeriupredoi valeriupredoi commented Jan 10, 2023

Description

As @bouweandela explains the root cause of a data finding error in ESMValGroup/ESMValCore#1866 (comment) it proves out that the old version Ed2-7 is obsolete, and is not found on ESGF no more, hence I am updating it; I'd like to keep the version facet so things are clean and open.


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.

New or updated recipe/diagnostic

@@ -48,7 +48,7 @@ diagnostics:
rsut: # TOA SW up all sky
preprocessor: pp_rad
additional_datasets:
- {dataset: CERES-EBAF, project: obs4MIPs, mip: Amon, level: L3B, version: Ed2-7, start_year: 2001, end_year: 2012, tier: 1}
- {dataset: CERES-EBAF, project: obs4MIPs, mip: Amon, level: L3B, version: v20160610, start_year: 2001, end_year: 2012, tier: 1}
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be simpler for maintenance purposes to remove the version of obs4MIPs datasets in cases like that? Otherwise, we would have to update the recipes every time an obs4MIPs dataset is retracted from ESGF.

Suggested change
- {dataset: CERES-EBAF, project: obs4MIPs, mip: Amon, level: L3B, version: v20160610, start_year: 2001, end_year: 2012, tier: 1}
- {dataset: CERES-EBAF, project: obs4MIPs, mip: Amon, level: L3B, start_year: 2001, end_year: 2012, tier: 1}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good to me, man! I thought we ought to give it all the times, but really, I don't have obs4mips OCD 😁

Copy link
Member

Choose a reason for hiding this comment

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

@remi-kazeroni In the future, I would propose to specify the version as much as possible for all datasets in all recipes. If a dataset is no longer publicly available the recipe is broken according to our policy because people cannot obtain the data and it should be dealt with according to that policy.

Co-authored-by: Rémi Kazeroni <70641264+remi-kazeroni@users.noreply.github.com>
@valeriupredoi
Copy link
Contributor Author

Nope, it looks like the old behaviour is still in - version is a needed key, that's probably changed now with @bouweandela 's versioning magic but that's still only in the dev branch of Core, gonna revert that commit 👍

@bouweandela
Copy link
Member

bouweandela commented Jan 16, 2023

< version is a needed key

Indeed, the default config-developer.yml of ESMValCore versions prior to the upcoming v2.8 uses the version facet to find the obs4MIPs data locally: https://github.com/ESMValGroup/ESMValCore/blob/v2.7.1/esmvalcore/config-developer.yml#L113

@valeriupredoi
Copy link
Contributor Author

Cheers @bouweandela - indeed, I too feel a bit more comfortable specifying the version, even if this means a bit more recipe maintenance. @remi-kazeroni you mind merging this, bud? 🍺

@remi-kazeroni
Copy link
Contributor

Cheers @valeriupredoi and @bouweandela 🍻

@remi-kazeroni remi-kazeroni merged commit 2f4bf4c into main Jan 19, 2023
@remi-kazeroni remi-kazeroni deleted the update_ob4mips-version_recipe_validation branch January 19, 2023 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants