Add ability to deal with masked arrays to find_intersections #372
Conversation
metpy/calc/tools.py
Outdated
| @@ -157,3 +158,35 @@ def interpolate_nans(x, y, kind='linear'): | |||
| else: | |||
| raise ValueError('Unknown option for kind: {0}'.format(str(kind))) | |||
| return y[x_sort_args] | |||
|
|
|||
|
|
|||
| @exporter.export | |||
There was a problem hiding this comment.
Given that this could go away in the future, I'm not ready to export this. I'd even consider naming with a leading underscore.
There was a problem hiding this comment.
Fair - Can do. Re above - I don't think we ever need to look back. nearest_intersection_idx shouldn't ever return a masked point.
jrleeman
force-pushed
the
LFC_Masked_Arrays
branch
2 times, most recently
from
587b78c
to
aab6a6d
Compare
|
Looks good to go unless you see anything else @dopplershift |
metpy/calc/tools.py
Outdated
| ------- | ||
| Index of next non-masked element and next non-masked element | ||
| """ | ||
| if hasattr(a, 'mask'): |
There was a problem hiding this comment.
Instead of checking here (LBYL-Look Before You Leap), use EAFP (Easier to Ask Forgiveness than Permission):
try:
...
except AttributeError:
return idx, a[idx]hasattr() ends up calling getattr() anyway.
There was a problem hiding this comment.
So is that an efficiency gain or mainly style thing?
There was a problem hiding this comment.
Ever so slight gain in efficiency has convinced me of choosing EAFP over LBYL; code clarity, of course, dominates all of that, but that's not the case here.
metpy/calc/tools.py
Outdated
| """ | ||
| if hasattr(a, 'mask'): | ||
| next_idx = idx | ||
| while ma.is_masked(a[next_idx]): |
There was a problem hiding this comment.
Instead of this loop, how about:
next_idx = idx + a[idx:].mask.argmin()
|
Looks like we have to catch all three errors for the entire build matrix to pass. If that sounds fine, I'll fixup the commits. |
|
Sounds good, rebase and we should be good to go. |
jrleeman
force-pushed
the
LFC_Masked_Arrays
branch
from
eff3625
to
d1eaf20
Compare
|
Done! I really hope we can kill this soon - it's not pretty, but sadly necessary. |
|
rebasing now so it can be next in line for merge |
jrleeman
force-pushed
the
LFC_Masked_Arrays
branch
from
d1eaf20
to
387c0d3
Compare
|
Ready to go |
Adds a helper function and streamlines its use in
find_intersections. It will need some tests of course, but let's start with talking about how we want to modify the inner workings.