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.

Sign up for GitHub

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

IMPRO-511 implement metadata cube saving #538

Merged
merged 18 commits into from
May 22, 2018
Merged

IMPRO-511 implement metadata cube saving #538

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
acf8cb4
Initial incorporation of metadata cube saving based on IMPRO-466/464.…
markysparks May 4, 2018
ceeb6be
Revert changes to spatial.py and test_DifferenceBetweenAdjacentGridSq…
markysparks May 4, 2018
f016ea3
Remove some print statements only used during testing
markysparks May 4, 2018
37d4383
Remove superfluous comment
markysparks May 4, 2018
c837506
Correct comment typo and remove spp__form_of_difference attribute fro…
markysparks May 8, 2018
2281ecc
Add unit test to check that metatdata prefixes cube is discarded duri…
markysparks May 8, 2018
784f7f9
Add unit tests for append_metadata_cube functionality
markysparks May 9, 2018
e8959e1
inital fix to keep global attributes in netCDF file, get required glo…
markysparks May 17, 2018
17c1e07
change iteritems() to items() for Python 3 compatability
markysparks May 17, 2018
752a220
Use a list of global keys as suggested by cgsandford
markysparks May 17, 2018
258fc3f
Pass in global_keys as an argument to append_metadata_cube so as to a…
markysparks May 17, 2018
a6b743f
Changes as per cgsandford suggestions, tests confirmed key value cann…
markysparks May 17, 2018
b8daf7d
amend comments
markysparks May 17, 2018
221a0b3
add unit test to check global attributes added to prefix cube, revert…
markysparks May 17, 2018
4e25fae
only use one cube in input list, compare global keys on input cube to…
markysparks May 21, 2018
ab9f53d
shorten line 223 length, travis build complaining about line length o…
markysparks May 21, 2018
b90d42d
add check that key values are also the same in prefix and data cube
markysparks May 21, 2018
688f102
Removed redundant conversion of cubelist to be a variable of type list.
gavinevans May 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion lib/improver/tests/utilities/test_save.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,16 @@ def set_up_test_cube():
""" Set up a temperature cube with additional global attributes. """
data = (np.linspace(-45.0, 45.0, 9).reshape(1, 1, 3, 3) + 273.15)
cube = set_up_cube(data, 'air_temperature', 'K', realizations=([0]))
cube.attributes['Conventions'] = 'CF-1.5'
cube.attributes['source_realizations'] = np.arange(12)
# Desired attributes that will be global in netCDF file
cube.attributes['Conventions'] = 'CF-1.5'
cube.attributes['grid_id'] = 'enukx_standard_v1'
cube.attributes['source'] = 'Met Office Unified Model'
cube.attributes['institution'] = 'Met Office'
cube.attributes['history'] = ''
cube.attributes['um_version'] = '10.4'
cube.attributes['title'] = 'Operational MOGREPS-UK Forecast Model'

return cube


Expand Down
28 changes: 28 additions & 0 deletions lib/improver/utilities/save.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,35 @@ def append_metadata_cube(cubelist):
Returns:
iris.cube.Cubelist with appended metadata cube
"""

keys_for_global_attr = {}

# Set up a basic prefix cube
prefix_cube = iris.cube.Cube(0, long_name='prefixes',
var_name='prefix_list')

# Iterate through the cubelist cubes attributes (using dictionary
# comprehension), collecting attributes that we want to be global
# attributes in a resulting netCDF file.
for cube in cubelist:
keys = cube.attributes
keys_for_global_attr = {k: v for k, v in keys.items()
if k is 'um_version'
or k is 'institution'
or k is 'source'
or k is 'grid_id'
or k is 'history'
or k is 'Conventions'
or k is 'title'}
Copy link
Contributor

@cgsandford cgsandford May 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

To avoid multiple definitions, could we pass in the list of global keys as an argument to append_metadata_cube? This would then simplify this code block to be:
keys_for_global_attr = {k for k in cube.attributes.keys() if k in global_keys}

Copy link
Contributor Author

@markysparks markysparks May 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

OK so I've just tried this out using a list of global keys in append_metadata_cube (which could or course be passed in as argument ultimately).

Your code seems to be producing a set rather than dictionary which caused problems when adding the keys to the prefix cube, so I added a line to produce a dictionary from the set which also set's the global key's value to ' '. I'm not sure if this is what you meant?

For the attributes to kept as global I'm not sure if the keys and the values of the attributes have to be the same or just the keys? (Its not clear to me from the Iris2 docs, but guess I'll find out when I generate some new data)

I'll push up this commit so you can see what you think.......

Copy link
Contributor

@cgsandford cgsandford May 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I think you could have it as I put it (returning a set), but then further down (at line 65) you need the following:

for key in keys_for_global_attr:
    prefix_cube.attributes[key] = cube.attributes[key]

That's slightly simpler, but probably doesn't matter if your code works.

However I do think it would be best if we didn't define the list of global keys twice. I'd recommend changing append_metadata_cube(cubelist) to append_metadata_cube(cubelist, global_keys), and passing in the list of global keys as defined in save_netcdf. An alternative would be to define GLOBAL_KEYS at the top of the file as a global variable, rather than locally within the functions, but this is not the preferred convention (see https://google.github.io/styleguide/pyguide.html#Global_variables).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think that would be the best way, passing in the global_keys list as an argument to append_metadata_cube.


# Attributes have to appear on all cubes in a cubelist for Iris 2 to save
# these attributes as global in a resulting netCDF file, so add all of the
# global attributes to the prefix cube (otherwise they will be made
# variables in the netCDF file).
for key, value in keys_for_global_attr.iteritems():
prefix_cube.attributes[key] = value
Copy link
Contributor

@cgsandford cgsandford May 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Ah actually, I think what this will do (when you create a set above) is add attributes to the prefix cube which have no value (as value is an empty string). Have you looked at an output cube to see what this does? I think you may need to assign using prefix_cube.attributes[key] = cube.attributes[key] to ensure the same global attributes are present.

Copy link
Contributor

@cgsandford cgsandford May 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

And maybe a unit test to check the right global attributes are added?


# Add metadata prefix attributes to the prefix cube
prefix_cube.attributes['spp__'] = \
'http://reference.metoffice.gov.uk/statistical-process/properties/'
prefix_cube.attributes['spv__'] = \
Expand All @@ -57,6 +84,7 @@ def append_metadata_cube(cubelist):

cubelist = list(cubelist)
cubelist.append(prefix_cube)
# bald__isPrefixedBy should be an attribute on all the cubes
for cube in cubelist:
cube.attributes['bald__isPrefixedBy'] = 'prefix_list'

Expand Down