search

hydrocode-de / RUINSapp

https://hydrocode-de.github.io/RUINSapp
MIT License
1 stars 1 forks source link

Future Warning #62

Open mmaelicke opened 2 years ago

mmaelicke commented 2 years ago

We have a new FutureWarning:

FutureWarning:

Units 'M', 'Y' and 'y' do not represent unambiguous timedelta values and will be removed in a future version

The traceback does not point to the actual causing code, but to streamlit, which is obviously not the problem. I guess I tracked it down to these lines:

https://github.com/hydrocode-de/RUINSapp/blob/06ad9d7f844bac517fea0ff94880159a9644c283/ruins/apps/weather.py#L160

mmaelicke commented 2 years ago

These are the xarray docs: https://xarray.pydata.org/en/stable/generated/xarray.Dataset.resample.html#id2

and they point to a pandas docs page, which moved to here: https://pandas.pydata.org/pandas-docs/stable/user_guide/timeseries.html

Searching for 'offset aliases' yields another link to this page: https://pandas.pydata.org/pandas-docs/stable/user_guide/timeseries.html#timeseries-offset-aliases

One of you can checkout the different aliases there, to see which are unambiguous. Alternatively, we can maybe create a datetime.timedelta object directly and pass that to _reduce_weather_data(time=<timedelta object>):

https://github.com/hydrocode-de/RUINSapp/blob/06ad9d7f844bac517fea0ff94880159a9644c283/ruins/apps/weather.py#L147

mmaelicke commented 2 years ago

Found it. It's this line:

https://github.com/hydrocode-de/RUINSapp/blob/06ad9d7f844bac517fea0ff94880159a9644c283/ruins/apps/weather.py#L271

pandas.Timedelta will not support the named shortcuts in future anymore. I'll fix myself...

mmaelicke commented 2 years ago

Actually, it's not that easy anymore. pd.Timedelta is internally storing timedeltas in nano-seconds. Thus, the month and year are removed, as they can't be expressed in a constant amount of nanoseconds.

@cojacoo, at the given line: https://github.com/hydrocode-de/RUINSapp/blob/06ad9d7f844bac517fea0ff94880159a9644c283/ruins/apps/weather.py#L271 you extract a 1 month chunk from the xarray. Is it ok to replace this by 30 days, which would be fine for pandas, or do we need to calculate the exact timedelta for the given month? That would involve to check the month of the first timestep and then creating a new one exactly one month eariler.

cojacoo commented 2 years ago

I have no clue if the removal of this functionality actually corrects a function which has never existed (doing all the checks etc.) or if it bluntly aggregated for 30 days anyways. For me using 30 days is fine. We could also use 30.5 days...

mmaelicke commented 2 years ago

I have no clue if the removal of this functionality actually corrects a function which has never existed (doing all the checks etc.) or if it bluntly aggregated for 30 days anyways. For me using 30 days is fine. We could also use 30.5 days...

Alright, makes my life easier. I'll append a fix to this issue to any of the upcoming versions.