-
Notifications
You must be signed in to change notification settings - Fork 77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Set EK60/80 Platform
and NMEA
nan timestamp to first ping_time
value [all tests ci]
#1154
Conversation
…o the first ping_time value. Addresses xarray warnings.
The saildrone EK80 data by default does not have the lat/lon data since GPS is not plugged into the echosounder itself, so those would be the example raw files. |
Platform.time1
to first ping_time
value when it's a single-valued nan [all tests ci]Platform.time1
to first ping_time
value when it's a single-valued nan [all tests ci]
Codecov Report
@@ Coverage Diff @@
## dev #1154 +/- ##
==========================================
+ Coverage 82.32% 82.35% +0.02%
==========================================
Files 64 64
Lines 5789 5803 +14
==========================================
+ Hits 4766 4779 +13
- Misses 1023 1024 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Platform.time1
to first ping_time
value when it's a single-valued nan [all tests ci]Platform
and NMEA
nan timestamp to first ping_time
value [all tests ci]
Ok, I ended up discovering more cases like this using the EK80 file. To summarize, these are:
Since they all can be handled in the same way, I added a new private method @emiliom: I changed your EK60 implementation to use the code I added for EK80, to account for cases where the channels may not ping simultaneously. I also changed the AZFP |
75cb2ca
to
3378e26
Compare
I'll self-merge this now to check everything is in good order for releasing v0.8.1. |
* add PRs * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * add #1154 * change #1154 PR title * change date; more info re 3.11 test failures * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * small wording tweak --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Addresses the first two xarray warnings identified in #1153. Implements the solution described there, for EK60.
Setting single-valued nan
Platform.time1
cases to the firstping_time
value is consistent with the approach currently used with AZFP. EK80 may benefit from this change, too, but I'm not aware of a test case raw file, plus we're short on time.See discussion in #1153.
[@leewujung edits for future reference]: Saildrone EK80 data by default does not have the lat/lon data since GPS is not plugged into the echosounder itself.