um_stash_source attribute causes ValueError if not in msi format

[jonseddon]

🐛 Bug Report

If a variable in a netCDF file contains a um_stash_source attribute that is not in MSI format mXXsXXiXXX (for example (28.97/48.0)*m01s34i001) then ValueError is raised when loading.

How To Reproduce

Steps to reproduce the behaviour:

import iris
from netCDF4 import Dataset
from iris.tests.stock import realistic_3d
cube = realistic_3d()
iris.save(cube, 'cube.nc')
rootgrp = Dataset('cube.nc', 'a')
apt = rootgrp.variables['air_potential_temperature']
apt.um_stash_source = '(28.97/48.0)*m01s34i001'
rootgrp.close()
cube = iris.load_cube('cube.nc')

Expected behaviour

ValueError seems excessive here. If the STASH code isn't in the correct format then perhaps a warning and storing the attribute value in cube.attributes would be nicer to users.

Environment

• OS & Version: RHEL7
• Iris Version: 3.0.0rc0

(but behaviour is probably OS and version independent)

Additional context

Fix probably needs to go into https://github.com/SciTools/iris/blob/master/lib/iris/fileformats/_pyke_rules/fc_rules_cf.krb#L1014 but I'm not sure how to handle an execption in Pyke myself.

[jonseddon]

@pp-mo

[bjlittle]

Hey @jonseddon, thanks for raising this issue.

Clearly you have a very specific use case here... so given that, I'm assuming that you as a users would want to hold onto the troublesome um_stash_source for whatever reason, hence your suggestion of adding it into the attributes dictionary, right?

[bjlittle]

Also, we're going to be transitioning to logging, so I'd err on the side of a logger debug message being issues rather than a warning... iris is noisy enough as it is 🙉 ... that would be appropriate, right?

[jonseddon]

Hey @jonseddon, thanks for raising this issue.

Clearly you have a very specific use case here... so given that, I'm assuming that you as a users would want to hold onto the troublesome um_stash_source for whatever reason, hence your suggestion of adding it into the attributes dictionary, right?

This came up in an internal discussion on Yammer. The attribute could be discarded, but I'm just always keen to not throw away stuff unnecessarily as in this case there is information inside it.

Logging would be an great improvement on a warning. Whether this should be at debug, info or warning should be carefully considered. Some users do rely on the STASH attribute and so if it's not going to be there then I'd opt for an info or warning level.

What's the ETA, or is there a branch/PR for the move to logging?

I'm happy to do the work on this. I've had a look through and I think that I can see how to check that the STASH is valid in the pyke rules. I'll create a draft PR as soon as poss.

[bjlittle]

@jonseddon I suspect that we may be releasing an iris 3.0.2 relatively soon in order to address issue #4000.

If you wanted to jump on this, then that would be fantastic and really helpful.

I suggest that you target your pull-request on the v3.0.x release feature branch and not master.

For the moment raise a suitable warning, that's totally fine for now, as it will be migrated to a logging message when we sweep through the code as part of that work.

Happy with that as a plan and way forward?

This means this fix will be available to the community as part of the forthcoming iris 3.0.2 release 👍

[bjlittle]

Note that the iris 3.0.x feature branch will require some commits cherry-picked from master to apply fixes to make cirrus-ci work... I'll deal with that for you 👍

So don't worry if you push a pull-request on v3.0.x and there are unrelated CI failures... a simple rebase may be required to resolve.

[bjlittle]

Closed by #4035