add cf names to conversions

Ouranosinc / xscen

A climate change scenario-building analysis framework.

https://xscen.readthedocs.io/

Apache License 2.0

15 stars 2 forks source link

add cf names to conversions #88

Closed RondeauG closed 2 years ago

RondeauG commented 2 years ago

Pull Request Checklist:

[x] pre-commit hooks are installed/active in my local clone ($ pre-commit install)
[x] This PR addresses an already opened issue (for bug fixes / features)
- This PR fixes #86
[x] (If applicable) Documentation has been added / updated (for bug fixes / features)
[x] If a merge request has been made in parallel to this PR in xscen-notebooks, it is merged and the submodules have been updated.
[x] HISTORY.rst has been updated (with summary of main changes)
- [x] Link to issue (:issue:number) and pull request (:pull:number) has been added

What kind of change does this PR introduce?

Where it was missing, added CF names to conversions.
Changed defaults in search_data_catalogs to force a more conscious choice when data is transformed.

Does this PR introduce a breaking change?

No.

Other information:

RondeauG commented 2 years ago

Do you think that there's an issue with relative_humidity and relative_humidity_from_dewpoint having the same variable name? I don't really have the data to test it out...

aulemahal commented 2 years ago

I don't believe so, but I can test it.

aulemahal commented 2 years ago

Woups yes it does, the registry is a dictionary where keys are the variable names. So only one of the hurs function gets registered, the last one in the yaml.

juliettelavoie commented 2 years ago

Donc, dans variable_and_freqs, on veut que les utilisateurs mettent ce qui est dans [cf_attrs][var_name], right? Si une personne demande hurs, lequel va être utilisé ? Il me semble que ça apporte de la confusion.

EDIT: oups j'avais pas vu ton commentaire Pascal.

RondeauG commented 2 years ago

Woups yes it does, the registry is a dictionary where keys are the variable names. So only one of the hurs function gets registered, the last one in the yaml.

Welp. In that case, I suggest keeping only one (relative_humidity_from_dewpoint ?) and exposing conversions.yml as an argument, so that people can provide their own version as needed. I think it's a decent compromise?

Edit: It would aslo solve the issue where if someone wants another method than Tetens30, this is currently not possible.