OSOceanAcoustics / echopype

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

Added support for `consolidate` subpackage functions to accept both in-memory or stored datasets #1216

Closed praneethratna closed 7 months ago

praneethratna commented 8 months ago

Addresses #1129, #1237 and now consolidate subpackage functions can accept both in-memory or stored datasets.

CC @leewujung

codecov-commenter commented 8 months ago

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (7679b96) 83.29% compared to head (bdee1cc) 62.43%. Report is 14 commits behind head on dev.

Files Patch % Lines
echopype/utils/io.py 55.55% 8 Missing :warning:
echopype/consolidate/api.py 91.66% 2 Missing :warning:
echopype/echodata/echodata.py 66.66% 1 Missing :warning:
echopype/mask/api.py 80.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #1216 +/- ## =========================================== - Coverage 83.29% 62.43% -20.87% =========================================== Files 64 24 -40 Lines 5675 1621 -4054 =========================================== - Hits 4727 1012 -3715 + Misses 948 609 -339 ``` | [Flag](https://app.codecov.io/gh/OSOceanAcoustics/echopype/pull/1216/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/1216/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OSOceanAcoustics) | `62.43% <76.47%> (-20.87%)` | :arrow_down: | 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.

praneethratna commented 7 months ago

Hey @leewujung I have addressed the above comments and also included the changes for #1237 in this PR since they are based on these changes. Thanks!

praneethratna commented 7 months ago

Hey @praneethratna : The last changes you made removed the part to save the added theta and phi variables to an existing dataset. Are you thinking that these variables would just be live in source_Sv?

Maybe we just remove the add_angle_to_ds function and put the content (with necessary modifications) in add_splitbeam_angle?

Hey @leewujung I think they should live, but anyway I have moved the content to add_splitbeam_angle itself and also removed the return_dataset parameter!