Add regrid mask functionality#1530
Conversation
ctuguinay
changed the title
ctuguinay
added
enhancement
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1530 +/- ##
==========================================
+ Coverage 85.58% 85.73% +0.15%
==========================================
Files 79 78 -1
Lines 6998 7016 +18
==========================================
+ Hits 5989 6015 +26
+ Misses 1009 1001 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
TODO: Try looking at using 1/0 with mean and if output strictly 1 and 0 for logical-AND then become 0 and if logical-OR then become 1. |
|
TODO: Add 2D range variable implementation and refactor the tests to work with it. |
| mask_regridded_da = xr.DataArray( | ||
| data=mask_regridded_da.data, | ||
| dims=[third_dim, "ping_time", range_da.name], | ||
| coords={ | ||
| third_dim: mask_da[third_dim].values, | ||
| "ping_time": np.array([v.left for v in mask_regridded_da.ping_time_bins.values]), | ||
| range_da.name: np.array( | ||
| [v.left for v in mask_regridded_da[f"{range_da.name}_bins"].values] | ||
| ), | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Here and also in the else case below, a new DataArray is constructed by taking the data in mask_regridded_da with new coordinates. I was wondering if it'll be more efficient to just rename the coordinates and update the values. But, looking at the I am not sure in practice if the underlying implementation - but it looks like a new object is created when rename is called anyway, so maybe it doesn't matter. Any thoughts here?
There was a problem hiding this comment.
Yeah I agree that it's cleaner to just rename and update values. I'll make that change.
leewujung
left a comment
leewujung
left a comment
There was a problem hiding this comment.
Hey @ctuguinay : Thanks for the PR! I think it's about ready to go - my inline comments are mostly just wording changes.
One thing I think that may be useful is to separate out the tests from test_mask.py to something more descriptive, now that we have so many tests in that module. We should split out the tests for seafloor and swarm detection, but maybe we do that when we know where the functions themselves would go (@LOCEANlloydizard and I were thinking they should probably go somewhere else instead of sitting in echopype.mask.
Co-authored-by: Wu-Jung Lee <leewujung@gmail.com>
for more information, see https://pre-commit.ci
Co-authored-by: Wu-Jung Lee <leewujung@gmail.com>
Co-authored-by: Wu-Jung Lee <leewujung@gmail.com>
for more information, see https://pre-commit.ci
Co-authored-by: Wu-Jung Lee <leewujung@gmail.com>
Co-authored-by: Wu-Jung Lee <leewujung@gmail.com>
Co-authored-by: Wu-Jung Lee <leewujung@gmail.com>
Co-authored-by: Wu-Jung Lee <leewujung@gmail.com>
Co-authored-by: Wu-Jung Lee <leewujung@gmail.com>
…me bin parsing and conversion as own util function, and rename and edit variables instead of creating new data array
for more information, see https://pre-commit.ci
|
Thanks for the changes @ctuguinay! I'll merge this now. |
4 participants
This resolves #1516.
The
ep.mask.regrid_maskfunction takes in axr.DataArrayand outputs axr.DataArray. Regridding is done using floxxarray_reduceand has two aggregation options:logical-ORand strictlogical-AND. These aggregations are not native to flox, so implement them, I turned 0/False into NaN and used flox built-innanmeanandmeanaggregations.