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 121 forks source link

Pin R <4.3.0 #3689

Closed ehogan closed 5 days ago

ehogan commented 5 days 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.


To help with the number of pull requests:

valeriupredoi commented 5 days ago

ine second - I just want to see if the pin impacts support for Python 3.12, brb

bouweandela commented 5 days ago

I am not terribly keen on introducing such a pin. We could do it for the v2.11.0 release, but should remove it again asap as this kind of pinning has given us a lot of work in the past.

valeriupredoi commented 5 days ago

I am not terribly keen on introducing such a pin. We could do it for the v2.11.0 release, but should remove it again asap as this kind of pinning has given us a lot of work in the past.

ha, I was posting the same while you were too - great :brain: think alike :laughing:

bouweandela commented 5 days ago

do you know anyone who knows R and would be willing to minimally maintain those recipes?

No

ehogan commented 5 days ago

I am not terribly keen on introducing such a pin. We could do it for the v2.11.0 release, but should remove it again asap as this kind of pinning has given us a lot of work in the past.

Yes, I agree, it's not ideal. However, once the ncdf4 package (which is the package I believe is causing the problems for the recipe_combined_indices.yml and recipe_extreme_events.yml recipes) has been upgraded to work with R>=4.3.0, the pin would no longer be needed ๐Ÿ‘

ehogan commented 5 days ago

Alternatively, we don't pin, I have a go at fixing fix the miles recipes, and we add the other two to the list of broken recipes?

valeriupredoi commented 5 days ago

OK merging this now, but we need to remove the pin as soon as it will be possible :+1: