Skip to content

Structured load api#2241

Merged
marqh merged 22 commits intoSciTools:masterfrom
pp-mo:structured_load_api
Nov 17, 2016
Merged

Structured load api#2241
marqh merged 22 commits intoSciTools:masterfrom
pp-mo:structured_load_api

Conversation

@pp-mo
Copy link
Member

@pp-mo pp-mo commented Nov 14, 2016

This now replaces the discussion PR on the "feature branch" in my repo : pp-mo#27

NOTE NOT READY YET : documentation changes still need to be reviewed/finalised/agreed.

from notes there:

As everything else was already agreed + merged to here, I've merged the outstanding pp-mo/iris#31 "Revised documentation, whatsnew and deprecations." here, as that is all that remains to discuss.
So I closed this and opened a new PR to the main Iris master instead : #2241
We can discuss what remains to be discussed there.

@pp-mo pp-mo force-pushed the structured_load_api branch from c641653 to fed341f Compare November 14, 2016 16:43
Header elements of potential concern include LBTIM, LBCODE, LBVC,
LBRSVD4 (ensemble number) and LBUSER5 (pseudo-level).

Known current shortcomings:
Copy link
Member Author

Choose a reason for hiding this comment

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

This section currently appears within the "warning" box, which may not be such a good idea.
It is not really about the same things.
On review, I'll investigate re-casting this as a separate "regular paragraph", like "Use of callbacks" and "Applicability"

The results from this are normally equivalent to those generated by
:func:`iris.load`, but the operation is substantially faster for input
which is structured.

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably don't need to separate these notes into their own paragraphs?

For calls other than :meth:`~iris.load_raw`, the resulting cubes are
concatenated over all the input files, so there is normally just one
output cube per phenomenon.

Copy link
Member Author

Choose a reason for hiding this comment

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

Again

Probably don't need to separate these notes into their own paragraphs?

inputs. Otherwise, odd behaviour and even incorrect loading can result, as
input files are not checked as fully as in a normal load.

Even where applicable, structured loading is not an *identical* replacement
Copy link
Member Author

Choose a reason for hiding this comment

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

"where applicable" --> "where it is applicable"
reads better

A context manager is used to enable fast um loading in all the regular iris
load functions, such as :meth:`iris.load` and :meth:`iris.load_cube`,
when loading data from UM file types.

Copy link
Member Author

Choose a reason for hiding this comment

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

Add a trivial example of usage.

dimensions and the choice of dimension coordinates.

* although both constraints and user callbacks are supported, callback
routines will generally need to be re-written.
Copy link
Member Author

Choose a reason for hiding this comment

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

Add brief words to explain the 'why+how' of the change to the callback context.

In particular, the callback's "field" argument is a
:class:`~iris.fileformats.um.FieldCollation`, from which "field.fields"
gives a *list* of PPFields from which that cube was built.
The code required is therefore different from a 'normal' callback.
Copy link
Member Author

Choose a reason for hiding this comment

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

Add a link to the test code, providing example of "translating" a callback.

For full details, see : :meth:`iris.fileformats.um.structured_um_loading`.

Not all input files will be suitable for structured loading: Each file
must have a regular repeating order of time and vertical levels, as described
Copy link
Member Author

Choose a reason for hiding this comment

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

Add the "complete set" concept into this statement somehow.
"Each input file must have a complete set of all combinations of time and vertical levels, arranged in a ordered pattern, as in UM model outputs."


For full details, see : :meth:`iris.fileformats.um.structured_um_loading`.

Not all input files will be suitable for structured loading: Each file
Copy link
Member Author

Choose a reason for hiding this comment

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

Add a hint that you can "try" structured load + compare, and in many cases results will be identical,.

]]
Fast UM file loading:
---------------------
Support has been added for accelerated loading of UM files (PP and
Copy link
Member Author

Choose a reason for hiding this comment

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

Add specific test-data example with speedup factor.

@pp-mo pp-mo force-pushed the structured_load_api branch from 9042b42 to 4d69994 Compare November 15, 2016 15:55
@pp-mo
Copy link
Member Author

pp-mo commented Nov 15, 2016

I think I've now addressed all the outstanding stuff we noted down.
Please re-review @marqh .

@marqh
Copy link
Member

marqh commented Nov 17, 2016

thank you for all the hard work @pp-mo

I agree that this is good to go, we have captured a few 'nice to have' elements in #2243
this code belongs on master now
any further changes will be additive

@marqh marqh merged commit ddb46f7 into SciTools:master Nov 17, 2016
@QuLogic QuLogic added this to the v1.12 milestone Nov 18, 2016
@pp-mo pp-mo deleted the structured_load_api branch January 3, 2017 12:51
@pp-mo pp-mo mentioned this pull request Jan 4, 2017
3 tasks
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