Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Future Warning #62

Open
mmaelicke opened this issue Apr 13, 2022 · 5 comments
Open

Future Warning #62

mmaelicke opened this issue Apr 13, 2022 · 5 comments
Assignees
Labels
bug Something isn't working priority: medium

Comments

@mmaelicke
Copy link
Member

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:

reduced = base.sel(vars=variable).resample(time=time)

@mmaelicke mmaelicke added the bug Something isn't working label Apr 13, 2022
@mmaelicke
Copy link
Member Author

mmaelicke commented Apr 13, 2022

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>):

def _reduce_weather_data(dataManager: DataManager, name: str, variable: str, time: str, station: str = None, _filter: dict = None) -> pd.DataFrame:

@mmaelicke
Copy link
Member Author

Found it. It's this line:

fig = yrplot_hm(pd.concat([wdata.loc[wdata.index[0]:data.index[0] - pd.Timedelta('1M')], data_ub.mean(axis=1)]), ref_yr, ag, li=2006, lang=config.lang)

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

@mmaelicke mmaelicke assigned mmaelicke and unassigned lucfr and AlexDo1 Apr 13, 2022
@mmaelicke
Copy link
Member Author

mmaelicke commented Apr 13, 2022

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:

fig = yrplot_hm(pd.concat([wdata.loc[wdata.index[0]:data.index[0] - pd.Timedelta('1M')], data_ub.mean(axis=1)]), ref_yr, ag, li=2006, lang=config.lang)

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
Copy link
Collaborator

cojacoo commented Apr 20, 2022

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
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority: medium
Projects
None yet
Development

No branches or pull requests

4 participants