OSOceanAcoustics / echopype

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

Enhanced `update_platform` to Auto-assign first `ping_time` as `Platform` timestamp for fixed-location update case without timestamp. [all tests ci] #1196

Closed praneethratna closed 10 months ago

praneethratna commented 10 months ago

Addresses #1126. and now update_platform will Auto-assign first ping_time as Platform timestamp for fixed-location update without a timestamp.

CC @leewujung @emiliom

codecov-commenter commented 10 months ago

Codecov Report

Merging #1196 (ee7bcf0) into dev (0bc534e) will increase coverage by 0.01%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev    #1196      +/-   ##
==========================================
+ Coverage   77.04%   77.05%   +0.01%     
==========================================
  Files          67       67              
  Lines        5908     5911       +3     
==========================================
+ Hits         4552     4555       +3     
  Misses       1356     1356              
Flag Coverage Δ
unittests 77.05% <100.00%> (+0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
echopype/echodata/echodata.py 77.88% <100.00%> (+0.22%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

praneethratna commented 10 months ago

Closing and re-opening the PR to rerun all the tests.

praneethratna commented 10 months ago

@praneethratna : Thank you for the PR! I only had a few small comments, and once those are cleaned up we can merge this.

@leewujung Resolved the comments. We can merge if the latest changes looks go to you.

And, seeing how long the .update_platform method is, I created another issue #1206 and assigned you to it. Hope it's ok. Thank you!!

Yes, I have seen the issue, will start working on it. Thanks!

leewujung commented 10 months ago

Looks great, thank you! I'll merge this now!