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

iris.util.unify_time_units changes time points dtype #3213

Closed
PAGWatson opened this issue Nov 5, 2018 · 4 comments
Closed

iris.util.unify_time_units changes time points dtype #3213

PAGWatson opened this issue Nov 5, 2018 · 4 comments

Comments

@PAGWatson
Copy link

Hello, I just spent quite a long time figuring out why two cubes would not concatenate. It turned out to be because I had used iris.util.unify_time_units on them, and this had turned the time coordinate points dtype for one cube from int64 to float64. This was very hard to track down because the dtype is not displayed when the coordinate is printed. I suggest that iris.util.unify_time_units also makes sure the coordinate dtypes are compatible.

@DPeterK
Copy link
Member

DPeterK commented Nov 26, 2018

Hi @PAGWatson - apologies for the delay in anyone responding to this issue. The unify_time_units utility function makes use of cf_units.Unit.convert_units to unify the time units as required. The convert_units method in turn uses functionality from udunits to perform the unit conversion, and this udunits operation can only operate with floating-point values. We could consider casting back to the incoming type at the end of the convert_units call, but that risks losing precision and would definitely be a breaking change in functionality, which delimits when such a change could be made to cf_units, if it were considered appropriate.

I'm also not sure that unify_time_units should be checking the dtype of the coordinate points. It could require another run around all the time coordinates being checked, making the function slower, and it's not really the job of unify_time_units to check the points of the coordinate associated with the unit. There's still the problem that some casting operations would result in decreased precision, so you might end up always casting up to the highest precision found, which may not be desirable.

That said, I assume that this issue relates to your comment in #2591, with which I quite agree - concatenate should be able to deal with differing dtypes as merge does. Getting that working would probably be a better solution than trying to retrofit something to unify_time_units, which was basically designed to ease the merge/concatenate process...

@rcomer
Copy link
Member

rcomer commented Sep 25, 2020

Hi @PAGWatson, do you know if this still happens for your use-case? A similar problem reported at #1036 seems to have been fixed some time before Iris 2.4 was released.

@PAGWatson
Copy link
Author

Thanks for following up. I'm afraid I can't remember what action gave me this error before, so I can't try to reproduce it right now.

@rcomer
Copy link
Member

rcomer commented May 1, 2021

I’m going to close this one, since there is reason to believe it may have been fixed and we don’t currently have a way to reproduce the problem. Obviously if anyone stumbles on the problem again, they can open a new issue, or we can re-open this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants