modelskill icon indicating copy to clipboard operation
modelskill copied to clipboard

support dataframes with localized timezones

Open otzi5300 opened this issue 2 years ago • 6 comments

Seems like it is not possible to create a comparer with dataframes with localized timezone.

Exception has occurred: TypeError '.dt' accessor only available for DataArray with datetime64 timedelta64 dtype or for arrays containing cftime datetime objects.

otzi5300 avatar Nov 09 '23 08:11 otzi5300

Uh, I guess we don't have any tests with that. I've never used localized time zones - for all the (marine) projects that I have done everything is UTC so we never worry about that. But we should make it work for sure - I easily see the relevance in the urban domain and other places.

@otzi5300 , could you start a PR on this? Maybe just adding a some small testdata and a failing test? Then we can take it from there...

jsmariegaard avatar Nov 09 '23 08:11 jsmariegaard

on it...

otzi5300 avatar Nov 09 '23 08:11 otzi5300

meanwhile, a work around is simply removing the timezones from the dataframes: df_mod.index = df_mod.index.tz_localize(None) df_obs.index = df_obs.index.tz_localize(None)

otzi5300 avatar Nov 09 '23 08:11 otzi5300

Can you explain the relevance of supporting timezones, isn't it enough to to use naive datetime (without tz info) and assume that all data is in the same timezone, similar to how we ignore coordinate reference system?

ecomodeller avatar Nov 09 '23 16:11 ecomodeller

Only supporting native datetime is ok but then I think the error msg should be made clearer. However, I think it is relatively common that sensor data are in a specific tz and I think we at least could support localized data in the same timezone.

otzi5300 avatar Nov 10 '23 10:11 otzi5300

🤔 both of your viewpoints seems fair. I guess it depends on the work needed to realize this. @ecomodeller is right, that we (for now) do not wish to support CRS (e.g. to allow some data in UTM-32 and other long-lat), but as @otzi5300 is pointing out, if it is commonly used, I think we should support tz-aware DatetimeIndex. At least if is not too much work

jsmariegaard avatar Nov 10 '23 17:11 jsmariegaard