OSOceanAcoustics / echopype

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

Add warnings and raised errors for `add_location` #1296

Closed ctuguinay closed 5 months ago

ctuguinay commented 6 months ago

Add warnings for add_location for NaN/0 lat/lon and duplicated timestamps to address the following issues: Strange GPS coordinates appearing with add_location() method #1286 add_location fails with duplicated timestamps in GPS data #1294

review-notebook-app[bot] commented 6 months ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

codecov-commenter commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 91.66%. Comparing base (9f56124) to head (c8e5ef5). Report is 41 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1296 +/- ## ========================================== + Coverage 83.52% 91.66% +8.14% ========================================== Files 64 3 -61 Lines 5686 168 -5518 ========================================== - Hits 4749 154 -4595 + Misses 937 14 -923 ``` | [Flag](https://app.codecov.io/gh/OSOceanAcoustics/echopype/pull/1296/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/1296/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OSOceanAcoustics) | `91.66% <100.00%> (+8.14%)` | :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 6 months ago

Oh wait, I'll add some tests to cover these scenarios.

ctuguinay commented 6 months ago

@leewujung This PR should be ready for review now

ctuguinay commented 5 months ago

@leewujung Made the switch from warning to raised value error. This should be ready for a quick review again.