Skip to content

Conversation

@pp-mo
Copy link
Member

@pp-mo pp-mo commented Dec 20, 2016

This should allow structured-loading to work with most files where pseudo-levels are used.

If 'ordinary' levels (LBLEV values) and 'pseudo-levels' (LBUSER5) both vary at once, this rather inelegant solution could still have problems, potentially producing 2 vertical coordinates.
But only if that occurs within a phenomenon, which we think is probably not realistic ?

Note:
Possibly needs tidying a bit further.
? Should I remove that commented-out 'fixed' failure code in tests.integration.fast_load.MixinProblemCases.test_FAIL_pseudo_levels ?
I have left it, assuming it is a useful statement of what the key problem actually is.

@pp-mo pp-mo changed the title Structured load api pseudolevels Structured load pseudolevels Dec 20, 2016
@pp-mo pp-mo changed the base branch from master to v1.12.x December 21, 2016 10:31
@pp-mo pp-mo force-pushed the structured_load_api_pseudolevels branch from efeab2b to 1d8954f Compare December 21, 2016 10:33
@pp-mo pp-mo added this to the v1.12.x milestone Dec 21, 2016
@marqh
Copy link
Member

marqh commented Dec 21, 2016

If 'ordinary' levels (LBLEV values) and 'pseudo-levels' (LBUSER5) both vary at once, this rather inelegant solution could still have problems, potentially producing 2 vertical coordinates.
But only if that occurs within a phenomenon, which we think is probably not realistic ?

I agree, this seems like a pretty edgy edge case to me

results = iris.load(file)
expected = CubeList(flds).merge()

# NOTE: this problem is now fixed : Structured load gives the same answer.
Copy link
Member

Choose a reason for hiding this comment

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

do we need to hold onto these notes explicitly in the source?
I'd have thought that this is what the git history is for
may we remove this?

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is that the version
Ok, I guess it is immortalised in the commit 33eb8d8
(though that won't be appearing in the main log).
I will tidy this up...

Copy link
Member

Choose a reason for hiding this comment

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

following an off line discussion about this I am persuaded to leave this in for now, as this commit will be squished so it will never be in teh history.

the handling of pseudo levels may require more effort, so these reminders may prove useful

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.

2 participants