Skip to content

Conversation

@DPeterK
Copy link
Member

@DPeterK DPeterK commented Jun 22, 2016

Fixes a bug (see #2052) where the IgnoreCubeException was not being respected in loading the loading of PP files with a user callback.

No tests yet, primarily because there are no tests for this behaviour at all (which is probably why this slipped through in the first place).

Closes #2052

@DPeterK
Copy link
Member Author

DPeterK commented Jun 22, 2016

I've now added a test! Its an integration test, because I couldn't figure out how to break the functionality of fileformats/rules.load_cubes down into unit-test-sized chunks. Also the behaviour caught here is integrated into the whole PP load framework, so I figure it fits into the integration tests.

@bjlittle bjlittle added this to the v1.10 milestone Jun 23, 2016
@DPeterK
Copy link
Member Author

DPeterK commented Jun 23, 2016

Hopefully this will get the tests passing...

@DPeterK
Copy link
Member Author

DPeterK commented Jun 23, 2016

ping @bjlittle - passing tests!

@bjlittle
Copy link
Member

@dkillick On it now ...

@bjlittle bjlittle self-assigned this Jun 23, 2016
# load if no format specific desired attributes are violated
if filter_function is None or filter_function(field):
yield (field, filename)

Copy link
Member

Choose a reason for hiding this comment

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

@dkillick This private functions doesn't appear to have changed, apart from its position in the file.

Sorry for being a pain, but could you paste it back, as it introduces unnecessary noise, thanks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops! Swap did occur 😒 Will shift it back!

loader.legacy_custom_rules.verify(cube, field)

# Then also run user-provided original callback function.
cube = iris.io.run_callback(user_callback, cube, field, filename)
Copy link
Member

Choose a reason for hiding this comment

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

@dkillick For me, this is the nub of the problem, and it's not your doing.

I believe lines +1034-1035 should now simply be the following:

result = cube
if user_callback is not None:
    result = user_callback(cube, field, filename)
return result

I believe the intent of the loadcubes_user_callback_wrapper was simply to

  1. raise the deprecation warning
  2. fire any registered custom user rules, then
  3. execute the user_callback.

The user_callback should not be executed here by iris.io.run_callback. That's the problem.

Rather, it is the loadcubes_user_callback_wrapper that should be executed by iris.io.run_callback downstream within _load_pairs_from_fields_and_filenames, as was prior to your changes.

So make the above change, then back-out your patch to _load_pairs_from_fields_and_filenames and replace it with

cube = iris.io.run_callback(user_callback, cube, field, filename)
if cube is None:
    continue

@bjlittle
Copy link
Member

@dkillick Great, thanks!

@bjlittle bjlittle merged commit 8ebf431 into SciTools:master Jun 23, 2016
@DPeterK DPeterK deleted the pp_callback branch June 24, 2016 08:08
agbutteriss added a commit to agbutteriss/iris that referenced this pull request Aug 4, 2016
agbutteriss added a commit to agbutteriss/iris that referenced this pull request Aug 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants