-
Notifications
You must be signed in to change notification settings - Fork 313
PP constraint #1237
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
PP constraint #1237
Changes from 4 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -647,7 +647,7 @@ def __iter__(self): | |
| return pp._interpret_fields(self._extract_field()) | ||
|
|
||
|
|
||
| def load_cubes(filenames, callback): | ||
| def load_cubes(filenames, callback, constraints): | ||
| """ | ||
| Loads cubes from a list of fields files filenames. | ||
|
|
||
|
|
@@ -667,10 +667,11 @@ def load_cubes(filenames, callback): | |
| orography references). | ||
|
|
||
| """ | ||
| return pp._load_cubes_variable_loader(filenames, callback, FF2PP) | ||
| return pp._load_cubes_variable_loader(filenames, callback, FF2PP, | ||
| constraints=constraints) | ||
|
|
||
|
|
||
| def load_cubes_32bit_ieee(filenames, callback): | ||
| def load_cubes_32bit_ieee(filenames, callback, constraints): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As elsewhere, |
||
| """ | ||
| Loads cubes from a list of 32bit ieee converted fieldsfiles filenames. | ||
|
|
||
|
|
@@ -680,4 +681,5 @@ def load_cubes_32bit_ieee(filenames, callback): | |
|
|
||
| """ | ||
| return pp._load_cubes_variable_loader(filenames, callback, FF2PP, | ||
| {'word_depth': 4}) | ||
| {'word_depth': 4}, | ||
| constraints=constraints) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1677,8 +1677,24 @@ def reset_save_rules(): | |
|
|
||
| _save_rules = None | ||
|
|
||
| def convert_constraints(constraints): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given its current exploratory nature, and hence the relatively high likelihood that its interface will change, it would be better if this function was kept private. |
||
| """ | ||
| Converts known constraints from Iris semantics to PP semantics | ||
| ignoring all unknown constraints. | ||
| """ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, it's super-picky but please could we have a blank line before the closing |
||
| constraints = iris._constraints.list_of_constraints(constraints) | ||
| pp_constraints = {} | ||
| for con in constraints: | ||
| if hasattr(con, '_attributes') and con._attributes.keys() == ['STASH']: | ||
| if not pp_constraints.has_key('stash'): | ||
| stashobj = STASH.from_msi(con._attributes['STASH']) | ||
| pp_constraints['stash'] = stashobj | ||
| else: | ||
| raise ValueError('multiple STASH constraints not supported' | ||
| 'by the PP loader. {}'.format(constraints)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's no need to raise an error here, nor is one wanted. When multiple STASH constraints are supplied the end result should be an empty "pp_constraints".
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like there's another logic issue here. Supplying a known constraint and an unknown constraint will currently result in only fields with the matching STASH code passing the pre-filter, thus excluding fields which might match the unknown constraint. Instead, whenever an unknown constraint is supplied the result of this function should be "allow everything".
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This could do with a test case. |
||
| return pp_constraints | ||
|
|
||
| def load_cubes(filenames, callback=None): | ||
| def load_cubes(filenames, callback=None, constraints=None): | ||
| """ | ||
| Loads cubes from a list of pp filenames. | ||
|
|
||
|
|
@@ -1688,6 +1704,8 @@ def load_cubes(filenames, callback=None): | |
|
|
||
| Kwargs: | ||
|
|
||
| * constraints - a list of Iris constraints | ||
|
|
||
| * callback - a function which can be passed on to :func:`iris.io.run_callback` | ||
|
|
||
| .. note:: | ||
|
|
@@ -1696,15 +1714,24 @@ def load_cubes(filenames, callback=None): | |
| is not preserved when there is a field with orography references) | ||
|
|
||
| """ | ||
| return _load_cubes_variable_loader(filenames, callback, load) | ||
| pp_constraints = {} | ||
| if constraints is not None: | ||
| pp_constraints = convert_constraints(constraints) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unused?! |
||
| return _load_cubes_variable_loader(filenames, callback, load, | ||
| constraints=constraints) | ||
|
|
||
|
|
||
| def _load_cubes_variable_loader(filenames, callback, loading_function, | ||
| loading_function_kwargs=None): | ||
| loading_function_kwargs=None, | ||
| constraints=None): | ||
| pp_constraints = {} | ||
| if constraints is not None: | ||
| pp_constraints = convert_constraints(constraints) | ||
| pp_loader = iris.fileformats.rules.Loader( | ||
| loading_function, loading_function_kwargs or {}, | ||
| iris.fileformats.pp_rules.convert, _load_rules) | ||
| return iris.fileformats.rules.load_cubes(filenames, callback, pp_loader) | ||
| return iris.fileformats.rules.load_cubes(filenames, callback, pp_loader, | ||
| pp_constraints) | ||
|
|
||
|
|
||
| def save(cube, target, append=False, field_coords=None): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -773,7 +773,7 @@ def _make_cube(field, converter): | |
| return cube, factories, references | ||
|
|
||
|
|
||
| def load_cubes(filenames, user_callback, loader): | ||
| def load_cubes(filenames, user_callback, loader, constraints={}): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Also see my comment about encapsulating the filter behaviour. |
||
| concrete_reference_targets = {} | ||
| results_needing_reference = [] | ||
|
|
||
|
|
@@ -783,13 +783,23 @@ def load_cubes(filenames, user_callback, loader): | |
| for filename in filenames: | ||
| for field in loader.field_generator(filename, **loader.field_generator_kwargs): | ||
| # Convert the field to a Cube. | ||
| cube, factories, references = _make_cube(field, loader.converter) | ||
|
|
||
| # Run any custom user-provided rules. | ||
| if loader.legacy_custom_rules: | ||
| loader.legacy_custom_rules.verify(cube, field) | ||
|
|
||
| cube = iris.io.run_callback(user_callback, cube, field, filename) | ||
| checked_field = field | ||
| # evaluate field against format specific constraints | ||
| # load if no format specific constraints are violated | ||
| for con_key, con_val in constraints.iteritems(): | ||
| if getattr(field, con_key) != con_val: | ||
| checked_field = None | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is overly complex for what is actually supported. Given the unknown future behaviour of this pre-filter idea I'd suggest encapsulating it to avoid it being split between this function and the For example: def load_cubes(filenames, user_callback, loader, filter_function=None):
# ...
if filter_function is not None and not filter_function(field):
continue
# ...NB. You don't need to use |
||
| if checked_field is not None: | ||
| cube, factories, references = _make_cube(checked_field, | ||
| loader.converter) | ||
|
|
||
| # Run any custom user-provided rules. | ||
| if loader.legacy_custom_rules: | ||
| loader.legacy_custom_rules.verify(cube, field) | ||
|
|
||
| cube = iris.io.run_callback(user_callback, cube, field, filename) | ||
| else: | ||
| cube = None | ||
|
|
||
| if cube is None: | ||
| continue | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -165,10 +165,11 @@ def expand_filespecs(file_specs): | |
| return sum(value_lists, []) | ||
|
|
||
|
|
||
| def load_files(filenames, callback): | ||
| def load_files(filenames, callback, constraints): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please make |
||
| """ | ||
| Takes a list of filenames which may also be globs, and optionally a | ||
| callback function, and returns a generator of Cubes from the given files. | ||
| constraint set and a callback function, and returns a | ||
| generator of Cubes from the given files. | ||
|
|
||
| .. note:: | ||
|
|
||
|
|
@@ -187,8 +188,13 @@ def load_files(filenames, callback): | |
|
|
||
| # Call each iris format handler with the approriate filenames | ||
| for handling_format_spec, fnames in handler_map.iteritems(): | ||
| for cube in handling_format_spec.handler(fnames, callback): | ||
| yield cube | ||
| if handling_format_spec.constraint_aware_handler: | ||
| for cube in handling_format_spec.handler(fnames, callback, | ||
| constraints): | ||
| yield cube | ||
| else: | ||
| for cube in handling_format_spec.handler(fnames, callback): | ||
| yield cube | ||
|
|
||
|
|
||
| def load_http(urls, callback): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| # (C) British Crown Copyright 2014, Met Office | ||
| # | ||
| # This file is part of Iris. | ||
| # | ||
| # Iris is free software: you can redistribute it and/or modify it under | ||
| # the terms of the GNU Lesser General Public License as published by the | ||
| # Free Software Foundation, either version 3 of the License, or | ||
| # (at your option) any later version. | ||
| # | ||
| # Iris is distributed in the hope that it will be useful, | ||
| # but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| # GNU Lesser General Public License for more details. | ||
| # | ||
| # You should have received a copy of the GNU Lesser General Public License | ||
| # along with Iris. If not, see <http://www.gnu.org/licenses/>. | ||
| """Unit tests for the `iris.fileformats.pp.load` function.""" | ||
|
|
||
| # Import iris.tests first so that some things can be initialised before | ||
| # importing anything else. | ||
| import iris.tests as tests | ||
|
|
||
| import mock | ||
|
|
||
| import iris | ||
| import iris.fileformats.pp as pp | ||
|
|
||
|
|
||
| class Test_convert_constraints(tests.IrisTest): | ||
| def test_single_stash(self): | ||
| constraint = iris.AttributeConstraint(STASH='m01s03i236') | ||
| pp_constraints = pp.convert_constraints(constraint) | ||
| expected_pp_cons = {'stash': pp.STASH.from_msi('m01s03i236')} | ||
| self.assertEqual(pp_constraints, expected_pp_cons) | ||
|
|
||
| def test_multiple_with_stash(self): | ||
| constraints = [iris.Constraint('air_potential_temperature'), | ||
| iris.AttributeConstraint(STASH='m01s00i004')] | ||
| pp_constraints = pp.convert_constraints(constraints) | ||
| expected_pp_cons = {'stash': pp.STASH.from_msi('m01s00i004')} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As mentioned elsewhere, I don't think this is the result you want. It will prevent the potential temperature fields from passing the pre-filter. |
||
| self.assertEqual(pp_constraints, expected_pp_cons) | ||
|
|
||
| def test_no_stash(self): | ||
| constraints = [iris.Constraint('air_potential_temperature'), | ||
| iris.AttributeConstraint(source='asource')] | ||
| pp_constraints = pp.convert_constraints(constraints) | ||
| expected_pp_cons = {} | ||
| self.assertEqual(pp_constraints, expected_pp_cons) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| tests.main() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| # (C) British Crown Copyright 2014, Met Office | ||
| # | ||
| # This file is part of Iris. | ||
| # | ||
| # Iris is free software: you can redistribute it and/or modify it under | ||
| # the terms of the GNU Lesser General Public License as published by the | ||
| # Free Software Foundation, either version 3 of the License, or | ||
| # (at your option) any later version. | ||
| # | ||
| # Iris is distributed in the hope that it will be useful, | ||
| # but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| # GNU Lesser General Public License for more details. | ||
| # | ||
| # You should have received a copy of the GNU Lesser General Public License | ||
| # along with Iris. If not, see <http://www.gnu.org/licenses/>. | ||
| """Unit tests for :func:`iris.fileformats.rules.load_cubes`.""" | ||
|
|
||
| # Import iris.tests first so that some things can be initialised before | ||
| # importing anything else. | ||
| import iris.tests as tests | ||
|
|
||
| import mock | ||
|
|
||
| import iris | ||
| from iris.fileformats import pp | ||
| from iris.fileformats.pp_rules import convert | ||
| from iris.fileformats.rules import load_cubes | ||
|
|
||
|
|
||
| class Test(tests.IrisTest): | ||
| def test_pp_with_stash_constraint(self): | ||
| filenames = [tests.get_data_path(('PP', 'globClim1', 'dec_subset.pp'))] | ||
| pp_constraints = {'stash': pp.STASH.from_msi('m01s00i004')} | ||
| pp_loader = iris.fileformats.rules.Loader(pp.load, {}, | ||
| convert, pp._load_rules) | ||
| cubes = list(load_cubes(filenames, None, pp_loader, pp_constraints)) | ||
| self.assertEqual(len(cubes), 38) | ||
|
|
||
| def test_pp_no_constraint(self): | ||
| filenames = [tests.get_data_path(('PP', 'globClim1', 'dec_subset.pp'))] | ||
| pp_constraints = {} | ||
| pp_loader = iris.fileformats.rules.Loader(pp.load, {}, | ||
| convert, pp._load_rules) | ||
| cubes = list(load_cubes(filenames, None, pp_loader, pp_constraints)) | ||
| self.assertEqual(len(cubes), 152) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| tests.main() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is in our public API so
constraintsshould be optional to make it backward compatible.