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

Modified attribute behaviour for netCDF save. #711

Merged
merged 3 commits into from
Aug 29, 2013

Conversation

esc24
Copy link
Member

@esc24 esc24 commented Aug 20, 2013

This PR modifies the treatment of cube attributes when saving to CF-netCDF. Currently cube attributes are saved as data variable attributes. However, this has the undesirable effect of routinely moving global CF attributes to data variable attributes when loading from, then saving to CF-netCDF. This PR modifies Iris so that it will save cube attributes as global, unless they are in a subset of CF-netCDF attributes that only apply to data variables (e.g. standard_error_multiplier). When saving multiple cubes to a single file, only cube attributes that are common across the collection will be stored as global, the rest will be stored as data variable attributes (with warnings being issued when saving global-only or data variable-only attributes in the wrong place).

_CF_GLOBAL_ATTRS = ['conventions', 'history', 'title']

# UKMO specific attributes that should not be global.
_UKMO_DATA_ATTRS = ['STASH', 'ukmo__um_stash_source', 'ukmo__process_flags']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'STASH' seems unnecessary here as netcdf saving turns STASH into ukmo__um_stash_source

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take it back. It turns out I did this for a reason. The cube.attributes dictionary key is 'STASH' not 'ukmo__um_stash_source'. I could in fact remove 'ukmo__um_stash_source' from this list, but I added it in case some one manually adds this attribute rather than using 'STASH'.

@marqh
Copy link
Member

marqh commented Aug 28, 2013

i would like to see an extra piece of functionality which enables a save operation to explicitly write global attributes to a file. This should be additive behaviour and can happily come in a new pull request

@marqh
Copy link
Member

marqh commented Aug 28, 2013

I would like to see the iris.save documentation updated to give a little more help to users

this can happily come in a new pull request

iris.save(self.cube, filename)
self.assertCDL(filename, ('netcdf', 'netcdf_save_attr.cdl'))

def test_conflicting_attributes(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this test really necessary? Are you not just testing dictionary behaviour here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry. Missed the cube2 part. Doh.

@pelson
Copy link
Member

pelson commented Aug 28, 2013

Generally I have no problems with this. One thing that I really struggle with (as a user) is figuring out what the behaviour of the NetCDF saver is supposed to be without actually reading and understanding the fairly complex code. For instance, it'd be great to have some documentation on the behaviour of NetCDF attribute saving somewhere. Any thoughts on that?

@esc24
Copy link
Member Author

esc24 commented Aug 28, 2013

Yes I agree. The save documentation needs an overhaul. @marqh is working on a PR.

@esc24
Copy link
Member Author

esc24 commented Aug 28, 2013

@pelson - I've another commits of review actions. Let me know what you think.

# that should be attributes on data variables.
attributes = cubes[0].attributes
common_keys = set(attributes)
for cube in cubes[1:]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic looks sound within this for loop (which is where all the complication relating to this PR exists). 👍

pelson added a commit that referenced this pull request Aug 29, 2013
Modified attribute behaviour for netCDF save.
@pelson pelson merged commit 2535bf8 into SciTools:master Aug 29, 2013
@pp-mo pp-mo mentioned this pull request Apr 18, 2023
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

Successfully merging this pull request may close these issues.

3 participants