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

Replace underscores in facets of observational datasets with minuses #3840

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

schlunma
Copy link
Contributor

@schlunma schlunma commented Dec 9, 2024

Description

This PR replaces _ in facets of observational datasets with -.

TODO:


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:

@schlunma schlunma added this to the v2.12.0 milestone Dec 9, 2024
@schlunma schlunma self-assigned this Dec 9, 2024
@schlunma schlunma requested a review from a team as a code owner December 9, 2024 14:22
Copy link
Contributor

@axel-lauer axel-lauer left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks @schlunma !

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.

cheers, Manu 🍺 I'll rename those on CEDA too 👍

@valeriupredoi
Copy link
Contributor

so I was faffing about looking at what datasets need filename changes on CEDA-JASMIN, and then it dawned to me: what if someone wants to use an older version of ESMValTool (like we support on JASMIN), wouldn't be better to have these both supported? ie wait for the DKRZ rsync that will actually bring in the "new" datasets, instead of renaming existing ones

@schlunma
Copy link
Contributor Author

what if someone wants to use an older version of ESMValTool (like we support on JASMIN), wouldn't be better to have these both supported? ie wait for the DKRZ rsync that will actually bring in the "new" datasets, instead of renaming existing ones

Yeah, the problem here is that we cannot have both versions if we want to use ESMValGroup/ESMValCore#1943 (which is the entire point of this little exercise here). If the old version is still there, facets will be assigned wrong.

I already thought about ways to make this fully backwards-compatible, but I couldn't think of one...The closest I got is creating an additional directory with links to the files (that use the old names), but this requires using a different configuration file.

@valeriupredoi
Copy link
Contributor

...which I can do easily for central installs; also - is the current naming (ie this PR) gonna work if we merge it now? I see that Core#1943 is still a draft

@schlunma
Copy link
Contributor Author

Yes, everything will work just fine. ESMValGroup/ESMValCore#1943 will enable reading facets off file names, which will for example allow finding the latest version of observational datasets.

...which I can do easily for central installs

Just out of curiosity - how do you do that? Changing the default configuration options?

@valeriupredoi
Copy link
Contributor

Just out of curiosity - how do you do that? Changing the default configuration options?

Yup: different installations grab different user config files, that users can then change too (not the base values for data dirs though) 🍺

I'll wait for the rsync: then I can just move the underscorey ones in a separate dir 👍

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.

OBS datasets use underscores in the version number
3 participants