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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve datum FUTURE warning #5749

Open
hdyson opened this issue Feb 16, 2024 · 3 comments
Open

Improve datum FUTURE warning #5749

hdyson opened this issue Feb 16, 2024 · 3 comments
Milestone

Comments

@hdyson
Copy link
Contributor

hdyson commented Feb 16, 2024

馃悰 Bug Report

When loading any netCDF file, a warning is produced from iris:

FutureWarning: Ignoring a datum in netCDF load for consistency with existing behaviour. In a future version of Iris, this datum will be applied. To apply the datum when loading, use the iris.FUTURE.datum_support flag.

The warning text implies that there is a datum in the file being loaded and it is being ignored. Can the behaviour to trigger the warning be aligned with the warning text, so the warning only fires if there is a datum in the source file?

This would make the warning much more useful for end users: users would only see the warning for those cases where there is a behaviour change controlled by the iris.FUTURE.datum_support flag (and hence where the user may need to understand or do something). At the moment, the warning is emitted for files where there will be no behaviour change too.

How To Reproduce

Steps to reproduce the behaviour:

  1. Load a netCDF file with iris 3.7 (or iris since 3.3ish?)

Expected behaviour

Only seeing the warning for netCDF files that do have a datum defined.

@pp-mo
Copy link
Member

pp-mo commented Feb 16, 2024

I reckon it is basically this code which is relevant to this.
I also don't understand why this chooses to issue a warning whenever a coordinate-system is built, and not just "if there is a datum attribute being ignored".

Unless there a good reason, I'd say this can very simply be changed by changing line 475
if not iris.FUTURE.datum_support: to if datum is not None and not iris.FUTURE.datum_support:

This code was added with #4704
Unfortunately, I'm not sure that either contributor or reviewer will be available to re-discuss this, so we may need to make our own decision!

@trexfeathers
Copy link
Contributor

This code was added with #4704
Unfortunately, I'm not sure that either contributor or reviewer will be available to re-discuss this, so we may need to make our own decision!

They can be responsive when asked! @wjbenfold @jamesp anything to add? No worries if you're busy 馃槉

@wjbenfold
Copy link
Contributor

Either I had some arcane reason to do this that's unfathomable to you guys, me and everyone else or I wrote a bug. I'm inclined to believe it's the latter, happy to throw up a PR to fix it as penance if you like but I dare say you'll beat me to it

@bjlittle bjlittle added this to the v3.9 milestone Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

5 participants