search

monocongo / climate_indices

Climate indices for drought monitoring
https://monocongo.github.io/climate_indices/
Other
353 stars 167 forks source link

Xarray 2023.1.0 dependency #535

Open matteodefelice opened 1 year ago

matteodefelice commented 1 year ago

Good evening, I am using your fantastic library for a project but I need a recent version of xarray because of a bug that has been solved. I was wondering if you plan to move to a more recent version of xarray or include a less restrictive dependency.

monocongo commented 1 year ago

Thanks for bringing this to my attention @matteodefelice

Yes, we can update the dependency to work around this bug. If you are comfortable doing so please update the pyproject.toml file accordingly and submit a PR. Otherwise please suggest a better version to use and I'll fix things myself. As long as the tests pass with a later version then we should be good to go.

matteodefelice commented 1 year ago

Just done a PR, I hope that requiring xarray 2023.10.0 is not too strict, otherwise we can find something in between.

monocongo commented 1 year ago

Seems like the PR will affect the ability to keep Python 3.8 support as that new version requires 3.9. What is the issue we're solving by using a more recent version of xarray? I'm happy to upgrade dependencies, but if we can do so without losing support for older Python versions that'll be preferable.

matteodefelice commented 1 year ago

Unfortunately (or not), xarray dropped the support to Python 3.8 at the beginning of the year. The bug seems associated to pandas: https://github.com/pydata/xarray/issues/2385

However, in my case, if I install xarray-2023.10 it disappears, reappearing when I go back to 2023.1 (pandas stays unchanged).

I understand that you don't want drop Python 3.8, in that case I will try to find another version of xarray (or pandas) compatible with python 3.8 that doesn't trigger the error.

monocongo commented 1 year ago

Thanks for sticking with it and looking for a 3.8-compatible solution. It looks like numpy dropped support for 3.8 this April so we're also facing that. Eventually, we'll fall in line.

WeatherGod commented 3 months ago

Why is the package pinning specific versions of dependencies? One should not be overly restrictive. Especially should not be specifying it down to the bugfix number, thereby preventing newer bugfix releases from getting used. These restrictions are preventing me from using newer versions of dask that supposedly fixed some chunking issues I'm having with the new ERA5 netcdf files.

monocongo commented 3 months ago

Yes, I agree. Pinning to major versions should be sufficient.

WeatherGod commented 3 months ago

And even that rule actually depends upon how that package does versioning. XArray's "major" version, for example, is the year of the release, but indicates nothing about compatibility.

WeatherGod commented 3 months ago

I'm doing some testing right now, but I should be able to put up an MR in a moment. Do we want to preemptively pin to numpy<2, or do you know already if numpy 2.0 works?

jamaa commented 2 months ago

@monocongo Can you please update the release on PyPI accordingly? The 2.0.0 release on PyPI still has the xarray dependency pinned to 2023.1.0, which is causing issues when installing from there.

monocongo commented 2 months ago

@jamaa I have looked into this today and it seems that 2023.1.0 is still the version poetry resolves to when I first use '*' as the version within pyproject.toml. My process for coming up with the correct version has been to set only the Python version and then run things until they break, adding in dependency packages first with a '*' (latest) version specified then iterating from there.

It appears that we're stuck on that version of xarray since we've kept support for Python 3.8. If I drop support for 3.8 then we get much later versions resolved by poetry.

3.8 is nearing its end-of-life date so probably we can follow suit.

jamaa commented 2 months ago

Is it not possible to specify xarray = '>=2023.1.0' as has already been merged in PR #557? Does that not preserve compatibility with Python 3.8 while also allowing newer versions of xarray if you want?

As it is right now, pip (which I am using) completely refuses to install a newer xarray version alongside climate_indices because it thinks that newer versions are not compatible at all. But at least in my applications, climate_indices also works with xarray 2024.6.0, which I need to use for other reasons.

monocongo commented 2 months ago

Yes, you're right. I have a fix already in place if I'll just release it accordingly. Thanks for your nudge in the right direction!

jamaa commented 2 months ago

Thank you very much, your work here is greatly appreciated

monocongo commented 2 months ago

I have just published version 2.0.1 on PyPI from the master branch code, please advise if there are snafus left to fix. Thanks for your help!

jamaa commented 2 months ago

with version 2.0.1, I can now install climate_indices alongside xarray 2024.6.0 without issues, thank you very much for the quick fix!