OSOceanAcoustics / echopype

Enabling interoperability and scalability in ocean sonar data analysis
https://echopype.readthedocs.io/
Apache License 2.0
89 stars 70 forks source link

Allow `MVBS` into `ep.mask.apply_mask`, and add Alignment Check for Target Variable and Mask #1345

Closed ctuguinay closed 6 days ago

ctuguinay commented 2 weeks ago

Addresses #1330

codecov-commenter commented 1 week ago

Codecov Report

Attention: Patch coverage is 89.47368% with 2 lines in your changes missing coverage. Please review.

Project coverage is 93.42%. Comparing base (9f56124) to head (aee9e12). Report is 95 commits behind head on main.

Files Patch % Lines
echopype/mask/api.py 89.47% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1345 +/- ## ========================================== + Coverage 83.52% 93.42% +9.90% ========================================== Files 64 3 -61 Lines 5686 213 -5473 ========================================== - Hits 4749 199 -4550 + Misses 937 14 -923 ``` | [Flag](https://app.codecov.io/gh/OSOceanAcoustics/echopype/pull/1345/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OSOceanAcoustics) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/OSOceanAcoustics/echopype/pull/1345/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OSOceanAcoustics) | `93.42% <89.47%> (+9.90%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OSOceanAcoustics#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ctuguinay commented 1 week ago

@leewujung This should be ready for review. It addresses the problem in that freq-diff and apply mask MVBS workflow that Soham showed the other day.

ctuguinay commented 1 week ago

@leewujung Thanks for the review!

So I suggest we do the following:

  • put these few lines directly in apply_mask

    # Validate the source_ds type or path (if it is provided)
    source_ds, file_type = validate_source(source_ds, storage_options_ds)
    
    if isinstance(source_ds, str):
      # open up Dataset using source_ds path
      source_ds = xr.open_dataset(source_ds, engine=file_type, chunks={}, **storage_options_ds)
  • rename the function to be _check_mask_dim_alignment for the new code you added

Yeah I agree, it was a bit much in a single function. I'll make these changes (once I get good WiFi again)