iris.save crashes for NetCDF for cube with no cell_methods

[volcan01010]

I have cube where cube.cell_methods == (None,). When I try to save it to NetCDF, Iris crashes with the following exception:

-> 1728             for name in cm.coord_names:
   1729                 coord = cube.coords(name)
   1730 

AttributeError: 'NoneType' object has no attribute 'coord_names'

The error occurs within the NetCDF Saver._create_cf_cell_methods() function.

I think this is a bug. Unfortunately I don't know enough about NetCDF to suggest a fix.

The following code reproduces the error using the NAME_No_time_averaging.txt file.

import iris
cubes = iris.load('NAME_No_time_averaging.txt')
iris.save(cubes, 'test.nc')

[pp-mo]

I have cube where cube.cell_methods == (None,)

That's not a save bug, it's a bad cube.

cube.cell_methods should definitely be an iterable, which should only contain iris.coords.CellMethods .
A "None" element is definitely not a viable option.

The question is, how did you come by such a thing ?

[volcan01010]

That's not a save bug, it's a bad cube.

OK. The cube.cell_methods tuple is iterable, but I didn't know enough about cell methods to know that None was not a valid item.

The cube was created from NAME output. The Name_No_time_averaging.txt file is a stripped-down example.

The None cell method is created by the _build_cell_methods() function of name_loader.py

iris/lib/iris/fileformats/name_loaders.py

Line 524 in 45a18ab

def _build_cell_methods(av_or_ints, coord):

The data in file matches the no_avg_pattern regular expression so the cell method is set to None.

[pp-mo]

@volcan01010 The data in file matches the no_avg_pattern regular expression so the cell method is set to None.

Ok, my apologies, that one clearly is a bug !!

The code seems in fact to have 2 different possible ways of getting cell_method == None,
so I reckon there is a simple "if cell_method is not None:" wanting here.
It looks like this NAME loader code is really old, and presumably we never had a test for this bit of behaviour.

I wonder, do you feel up to proposing a fix for this yourself ?
It should be pretty simple, but it could do with a test or two to cover it.
I quite understand if not, but it would be handy,
as if you have some understanding of the file format, then I certainly don't + I'm not sure who here now does ...

[volcan01010]

Yes, I can make a fix. I'll have a go next week. I have more questions; I'll ask them then.

[pp-mo]

Fantastic! Thanks so much for engaging 💐 🥇

[volcan01010]

I can have a go at fixing this (including tests) but I'm not sure what the correct behaviour is.

Should I update the NAME loader so that it doesn't return a cell_method of None? In this case, what should it do instead where there is no time averaging?

Alternatively, should I update the NetCDF Saver so that it doesn't crash if it receives a cell_method of None? In this case, what should it do instead?

[benjamindrummond]

Hi - just wondering if anything came of this? I've just re-encountered the problem (iris v3.0.1), so I am assuming not. Same issue, lloading NAME data that has no time averaging, and then I need to save the cube to NetCDF. Thanks

[DPeterK]

@benjamindrummond it sure does look like this has stalled...

The correct behaviour for Iris cubes without cell methods, incidentally, is that their cell_methods variable should be set to an empty tuple (). We can use this as a workaround:

# Confirm current behaviour.
print(cube.cell_methods)
(None,)

# Correctly specify that this cube doesn't have cell methods.
cube.cell_methods = ()

This workaround should mean you can at least save affected cubes!

[pp-mo]

I think I've now got #4933 into a reasonable state.
Does anyone from here feel like checking that out ? @volcan01010 @DPeterK @benjamindrummond

I have noted in #4933 that we could add an integration test for the fix, but it needs suitable test data which I don't have the knowledge to be sure of.
If anyone thinks that testing the fix is important, I just need a little more help with that : In the simplest way, @volcan01010 you could just confirm that I can add your testfile into iris-test-data ?
But I will only add this if someone thinks it is important.

[volcan01010]

Yes, please use the test file in iris-test-data.

[pp-mo]

Yes, please use the test file in iris-test-data.
Ok @volcan01010 thanks !!

I will add a testcase for that, and fix the outstanding merge conflict ...

[pp-mo]

Ok I think #4933 fixed!

[trexfeathers]

Ok I think #4933 fixed!

Just need to make sure we merge it to main 😉