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

Update the name of the remapcon2 operator in R recipes #3611

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.

bouweandela commented 1 month ago

Thanks! Could you please do the checklist?

ehogan commented 1 month ago

Thanks! Could you please do the checklist?

Apologies, now done! ๐Ÿ˜Š Although, it did make me wonder whether I should add a pin to cdo (>=2.3.0) as remapscon2 will only work from v2.3.0 onwards ๐Ÿค”

bouweandela commented 1 month ago

Thanks, that's why we have the checklist ;-) Updating the pin sounds like a good idea.

ehogan commented 1 month ago

Thanks, that's why we have the checklist ;-) Updating the pin sounds like a good idea.

Thanks @bouweandela! ๐Ÿฅณ Added in 618b6f8 ๐Ÿ‘

valeriupredoi commented 1 month ago

@ehogan cdo on PyPI is old, stuck at 1.6.0 so pinning it in setup.py will trigger an actual search of the index, so it fails coz it can't find a requested version to satisfy the pin, I'd not pin it in setup.py, and just leave it pinned in the conda env file ๐Ÿ‘๐Ÿบ

ehogan commented 1 month ago

@ehogan cdo on PyPI is old, stuck at 1.6.0 so pinning it in setup.py will trigger an actual search of the index, so it fails coz it can't find a requested version to satisfy the pin, I'd not pin it in setup.py, and just leave it pinned in the conda env file ๐Ÿ‘๐Ÿบ

Thanks @valeriupredoi; I removed the pin in setup.py in b929755. I also realised I hadn't added it to environment_osx.yml, so I did that in bfe68d9 ๐Ÿ˜Š

valeriupredoi commented 1 month ago

Many thanks @ehogan all looks good to me, @bouweandela maybe you could approve and merge when you got a minute ๐Ÿบ

bouweandela commented 1 month ago

cdo on PyPI is old, stuck at 1.6.0 so pinning it in setup.py will trigger an actual search of the index, so it fails coz it can't find a requested version to satisfy the pin, I'd not pin it in setup.py, and just leave it pinned in the conda env file

That is not quite correct. The cdo package on PyPI is on conda-forge as python-cdo. This is a Python wrapper around cdo, not cdo itself. See https://github.com/conda-forge/python-cdo-feedstock/blob/main/recipe/meta.yaml.