-
Notifications
You must be signed in to change notification settings - Fork 300
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
Conversation
|
👍 for the overall idea. It's something we've been kicking around for a long time, so it's nice to it finally come to pass in a simple, targeted form. |
lib/iris/fileformats/pp.py
Outdated
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.
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".
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.
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".
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.
When multiple STASH constraints are supplied the end result should be an empty "pp_constraints".
This could do with a test case.
Just to emphasise the benefit of @marqh's work here ... this makes a huge difference when picking a single STASH code out of a large collections of PP fields. @marqh has demonstrated a 20x speed-up for a real-world example. 😀 |
|
refactored to implement a field filter function as suggested |
🎉 |
|
We should target v1.7.x with this as a way to combat some of the slowdowns that seem to have crept in. |
|
👍 from me too! Tests show reading in a pp field from a list of files which contain several fields is sped up by a factor of 12. This is the sort of a gain in performance that would really encourage users to take up Iris. |
|
replaced by #1240 to target 1.7.x |
Speed up for PP and FieldsFiles where a constraint:
is used.