Skip to content

Improve gracefully filling warning #3171

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

Merged
DPeterK merged 6 commits into from
Sep 28, 2018
Merged

Improve gracefully filling warning #3171

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4a166cd
Realise that running the tests is simpler than I thought
znicholls Sep 16, 2018
52331d2
Make Gracefully warning only appear when filling is required
znicholls Sep 16, 2018
be08462
Add tests of warnings
znicholls Sep 16, 2018
7f0e676
Get rid of install changes, they are in #3170
znicholls Sep 17, 2018
9c56b8a
Update after review
znicholls Sep 18, 2018
c86b5a7
Add whatsnew entry
znicholls Sep 19, 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
82 changes: 41 additions & 41 deletions lib/iris/fileformats/_pyke_rules/fc_rules_cf.krb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ fc_default

#
# Context:
# This rule will trigger iff a grid_mapping() case specific fact
# This rule will trigger iff a grid_mapping() case specific fact
# has been asserted that refers to a rotated pole.
#
# Purpose:
Expand Down Expand Up @@ -120,7 +120,7 @@ fc_provides_grid_mapping_mercator

#
# Context:
# This rule will trigger iff a grid_mapping() case specific fact
# This rule will trigger iff a grid_mapping() case specific fact
# has been asserted that refers to a stereographic.
#
# Purpose:
Expand Down Expand Up @@ -199,10 +199,10 @@ fc_provides_grid_mapping_albers_equal_area
#
# Context:
# This rule will trigger iff a coordinate() case specific fact
# has been asserted that refers to a CF latitude coordinate.
# has been asserted that refers to a CF latitude coordinate.
#
# Purpose:
# Assert that the CF latitude coordinate exists.
# Assert that the CF latitude coordinate exists.
#
fc_provides_coordinate_latitude
foreach
Expand Down Expand Up @@ -435,10 +435,10 @@ fc_build_auxiliary_coordinate_longitude_rotated
#
# Context:
# This rule will trigger for each auxiliary_coordinate() case specific fact
# that is not a spatio-temporal related auxiliary coordinate.
# that is not a spatio-temporal related auxiliary coordinate.
#
# Purpose:
# Add the auxiliary coordinate to the cube.
# Add the auxiliary coordinate to the cube.
#
fc_build_auxiliary_coordinate
foreach
Expand All @@ -457,7 +457,7 @@ fc_build_auxiliary_coordinate
# This rule will trigger for each cell_measure case specific fact.
#
# Purpose:
# Add the cell measures attribute to the cube.
# Add the cell measures attribute to the cube.
#
fc_build_cell_measure
foreach
Expand Down Expand Up @@ -549,7 +549,7 @@ fc_build_coordinate_longitude_rotated
python build_dimension_coordinate(engine, cf_coord_var,
coord_name=CF_VALUE_STD_NAME_GRID_LON,
coord_system=engine.provides['coordinate_system'])
python engine.rule_triggered.add(rule.name)
python engine.rule_triggered.add(rule.name)


#
Expand Down Expand Up @@ -597,7 +597,7 @@ fc_build_coordinate_longitude_nocs
coord_system=None)
python engine.rule_triggered.add(rule.name)


#
# Context:
# This rule will trigger iff a projection_x_coordinate coordinate exists and
Expand Down Expand Up @@ -917,8 +917,8 @@ fc_attribute_ukmo__process_flags
python attr_value = engine.cf_var.ukmo__process_flags
python engine.cube.attributes['ukmo__process_flags'] = tuple([x.replace("_", " ") for x in attr_value.split(" ")])
python engine.rule_triggered.add(rule.name)


#
# Context:
# This rule will trigger iff a formula term that refers to a
Expand Down Expand Up @@ -1076,8 +1076,8 @@ fc_extras
import iris.std_names
import iris.util
from iris._lazy_data import as_lazy_data


#
# UD Units Constants (based on Unidata udunits.dat definition file)
#
Expand All @@ -1087,7 +1087,7 @@ fc_extras
UD_UNITS_LON = ['degrees_east', 'degree_east', 'degree_e', 'degrees_e',
'degreee', 'degreese', 'degrees', 'degrees east',
'degree east', 'degree e', 'degrees e']

#
# CF Dimensionless Vertical Coordinates
#
Expand Down Expand Up @@ -1119,7 +1119,7 @@ fc_extras
CF_GRID_MAPPING_STEREO = 'stereographic'
CF_GRID_MAPPING_TRANSVERSE = 'transverse_mercator'
CF_GRID_MAPPING_VERTICAL = 'vertical_perspective'

#
# CF Attribute Names.
#
Expand Down Expand Up @@ -1149,7 +1149,7 @@ fc_extras
CF_ATTR_LONG_NAME = 'long_name'
CF_ATTR_UNITS = 'units'
CF_ATTR_CELL_METHODS = 'cell_methods'

#
# CF Attribute Value Constants.
#
Expand All @@ -1158,11 +1158,11 @@ fc_extras
CF_VALUE_AXIS_Y = 'y'
CF_VALUE_AXIS_T = 't'
CF_VALUE_AXIS_Z = 'z'


# Attribute - positive.
CF_VALUE_POSITIVE = ['down', 'up']

# Attribute - standard_name.
CF_VALUE_STD_NAME_LAT = 'latitude'
CF_VALUE_STD_NAME_LON = 'longitude'
Expand Down Expand Up @@ -1213,7 +1213,7 @@ fc_extras
msg = msg.replace('variable', 'variable {!r}'.format(name))
warnings.warn(message=msg, category=UnknownCellMethodWarning)

# Set the cube global attributes.
# Set the cube global attributes.
for attr_name, attr_value in six.iteritems(cf_var.cf_group.global_attributes):
try:
if six.PY2 and isinstance(attr_value, six.text_type):
Expand Down Expand Up @@ -1242,7 +1242,7 @@ fc_extras

# Check for a default spherical earth.
if major is None and minor is None and inverse_flattening is None:
major = getattr(cf_grid_var, CF_ATTR_GRID_EARTH_RADIUS, None)
major = getattr(cf_grid_var, CF_ATTR_GRID_EARTH_RADIUS, None)

return major, minor, inverse_flattening

Expand Down Expand Up @@ -1344,7 +1344,7 @@ fc_extras
inverse_flattening is not None:
ellipsoid = iris.coord_systems.GeogCS(major, minor,
inverse_flattening)

cs = iris.coord_systems.LambertConformal(
latitude_of_projection_origin, longitude_of_central_meridian,
false_easting, false_northing, standard_parallel,
Expand Down Expand Up @@ -1401,7 +1401,7 @@ fc_extras
# values for false_easting, false_northing,
# scale_factor_at_projection_origin and standard_parallel. These are
# checked elsewhere.

ellipsoid = None
if major is not None or minor is not None or \
inverse_flattening is not None:
Expand Down Expand Up @@ -1621,7 +1621,7 @@ fc_extras
attr_units = get_attr_units(cf_coord_var, attributes)
points_data = cf_coord_var[:]
# Gracefully fill points masked array.
if ma.isMaskedArray(points_data):
if (ma.isMaskedArray(points_data)) and (not (points_data.mask == False).all()):
points_data = ma.filled(points_data)
msg = 'Gracefully filling {!r} dimension coordinate masked points'
warnings.warn(msg.format(str(cf_coord_var.cf_name)))
Expand All @@ -1631,7 +1631,7 @@ fc_extras
if cf_bounds_var is not None:
bounds_data = cf_bounds_var[:]
# Gracefully fill bounds masked array.
if ma.isMaskedArray(bounds_data):
if (ma.isMaskedArray(bounds_data)) and (not (bounds_data.mask == False).all()):
bounds_data = ma.filled(bounds_data)
msg = 'Gracefully filling {!r} dimension coordinate masked bounds'
warnings.warn(msg.format(str(cf_coord_var.cf_name)))
Expand Down Expand Up @@ -1696,7 +1696,7 @@ fc_extras
else:
# Scalar coords are placed in the aux_coords container.
cube.add_aux_coord(coord, data_dims)

# Update the coordinate to CF-netCDF variable mapping.
engine.provides['coordinates'].append((coord, cf_coord_var.cf_name))

Expand Down Expand Up @@ -1739,7 +1739,7 @@ fc_extras
# and the coordinate being built.
common_dims = [dim for dim in cf_coord_var.dimensions
if dim in cf_var.dimensions]
data_dims = None
data_dims = None
if common_dims:
# Calculate the offset of each common dimension.
data_dims = [cf_var.dimensions.index(dim) for dim in common_dims]
Expand All @@ -1763,7 +1763,7 @@ fc_extras
# Update the coordinate to CF-netCDF variable mapping.
engine.provides['coordinates'].append((coord, cf_coord_var.cf_name))


################################################################################
def build_cell_measures(engine, cf_cm_attr, coord_name=None):
"""Create a CellMeasure instance and add it to the cube."""
Expand All @@ -1780,19 +1780,19 @@ fc_extras
# and the coordinate being built.
common_dims = [dim for dim in cf_cm_attr.dimensions
if dim in cf_var.dimensions]
data_dims = None
data_dims = None
if common_dims:
# Calculate the offset of each common dimension.
data_dims = [cf_var.dimensions.index(dim) for dim in common_dims]

# Determine the standard_name, long_name and var_name
standard_name, long_name, var_name = get_names(cf_cm_attr, coord_name, attributes)

# Obtain the cf_measure.
measure = cf_cm_attr.cf_measure

# Create the CellMeasure
cell_measure = iris.coords.CellMeasure(data,
cell_measure = iris.coords.CellMeasure(data,
standard_name=standard_name,
long_name=long_name,
var_name=var_name,
Expand All @@ -1802,17 +1802,17 @@ fc_extras

# Add it to the cube
cube.add_cell_measure(cell_measure, data_dims)



################################################################################
def _is_lat_lon(cf_var, ud_units, std_name, std_name_grid, axis_name, prefixes):
"""
Determine whether the CF coordinate variable is a latitude/longitude variable.

Ref: [CF] Section 4.1 Latitude Coordinate.
[CF] Section 4.2 Longitude Coordinate.

"""
is_valid = False
attr_units = getattr(cf_var, CF_ATTR_UNITS, None)
Expand Down Expand Up @@ -1854,7 +1854,7 @@ fc_extras
def is_latitude(engine, cf_name):
"""Determine whether the CF coordinate variable is a latitude variable."""
cf_var = engine.cf_var.cf_group[cf_name]
return _is_lat_lon(cf_var, UD_UNITS_LAT, CF_VALUE_STD_NAME_LAT,
return _is_lat_lon(cf_var, UD_UNITS_LAT, CF_VALUE_STD_NAME_LAT,
CF_VALUE_STD_NAME_GRID_LAT, CF_VALUE_AXIS_Y, ['lat', 'rlat'])


Expand Down Expand Up @@ -1910,7 +1910,7 @@ fc_extras
is_time_reference = cf_units.Unit(attr_units or 1).is_time_reference()
except ValueError:
is_time_reference = False

return is_time_reference and (attr_std_name=='time' or attr_axis.lower()==CF_VALUE_AXIS_T)


Expand Down Expand Up @@ -1978,7 +1978,7 @@ fc_extras
def has_supported_mercator_parameters(engine, cf_name):
"""Determine whether the CF grid mapping variable has the supported
values for the parameters of the Mercator projection."""

is_valid = True
cf_grid_var = engine.cf_var.cf_group[cf_name]

Expand Down Expand Up @@ -2018,8 +2018,8 @@ fc_extras
################################################################################
def has_supported_stereographic_parameters(engine, cf_name):
"""Determine whether the CF grid mapping variable has a value of 1.0
for the scale_factor_at_projection_origin attribute."""
for the scale_factor_at_projection_origin attribute."""

is_valid = True
cf_grid_var = engine.cf_var.cf_group[cf_name]

Expand Down Expand Up @@ -2102,6 +2102,6 @@ fc_extras
if len(d[CM_NAME]) != len(comment) and len(comment) == 1:
comment = comment*len(d[CM_NAME])
d[CM_INTERVAL] = tuple(interval)
d[CM_COMMENT] = tuple(comment)
d[CM_COMMENT] = tuple(comment)
cell_methods.append(iris.coords.CellMethod(d[CM_METHOD], coords=d[CM_NAME], intervals=d[CM_INTERVAL], comments=d[CM_COMMENT]))
return tuple(cell_methods)
56 changes: 55 additions & 1 deletion ...nit/fileformats/pyke_rules/compiled_krb/fc_rules_cf_fc/test_build_dimension_coordinate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# (C) British Crown Copyright 2014 - 2015, Met Office
# (C) British Crown Copyright 2014 - 2018, Met Office
#
# This file is part of Iris.
#
Expand Down Expand Up @@ -112,6 +112,60 @@ def test_dim_coord_construction(self):
self.engine.cube.add_dim_coord.assert_called_with(
expected_coord, [0])

def test_dim_coord_construction_masked_array(self):
self._set_cf_coord_var(np.ma.array(
np.arange(6),
mask=[True, False, False, False, False, False],
fill_value=-999,
))

expected_coord = DimCoord(
np.array([-999, 1, 2, 3, 4, 5]),
long_name=self.cf_coord_var.long_name,
var_name=self.cf_coord_var.cf_name,
units=self.cf_coord_var.units,
bounds=self.bounds)

with warnings.catch_warnings(record=True) as w:
# Asserts must lie within context manager because of deferred
# loading.
with self.deferred_load_patch, self.get_cf_bounds_var_patch:
build_dimension_coordinate(self.engine, self.cf_coord_var)

# Test that expected coord is built and added to cube.
self.engine.cube.add_dim_coord.assert_called_with(
expected_coord, [0])

# Assert warning is raised
assert len(w) == 1
assert 'Gracefully filling' in w[0].message.args[0]

def test_dim_coord_construction_masked_array_mask_does_nothing(self):
self._set_cf_coord_var(np.ma.array(
np.arange(6),
mask=False,
))

expected_coord = DimCoord(
self.cf_coord_var[:],
long_name=self.cf_coord_var.long_name,
var_name=self.cf_coord_var.cf_name,
units=self.cf_coord_var.units,
bounds=self.bounds)

with warnings.catch_warnings(record=True) as w:
# Asserts must lie within context manager because of deferred
# loading.
with self.deferred_load_patch, self.get_cf_bounds_var_patch:
build_dimension_coordinate(self.engine, self.cf_coord_var)

# Test that expected coord is built and added to cube.
self.engine.cube.add_dim_coord.assert_called_with(
expected_coord, [0])

# Assert no warning is raised
assert len(w) == 0
Copy link
Member

@DPeterK DPeterK Sep 18, 2018

Choose a reason for hiding this comment

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

These new tests look great – but would you mind adding testing that covers the cases where the incoming coordinate has masked bounds? Thanks!

Copy link
Contributor Author

@znicholls znicholls Sep 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

have a look now, I think this is what you wanted?


def test_aux_coord_construction(self):
# Use non monotonically increasing coordinates to force aux coord
# construction.
Expand Down