-
Notifications
You must be signed in to change notification settings - Fork 30
ENH/SE: don't deadreckon position #218
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a deadreckon
flag to raw_to_timeseries
, updates tests and example scripts to exercise both modes, and adjusts example YAML to use surface latitude/longitude (Lat
/Lon
) instead of dead-reckoned fields.
- Introduce
deadreckon=False
parameter inraw_to_timeseries
to ignore underwater dead-reckoning jumps - Update tests and example scripts to pass
deadreckon
flag explicitly - Change example deployment YAML to reference
Lat
/Lon
as sources for latitude and longitude
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
tests/test_pyglider.py | Pass deadreckon=True to raw_to_L0timeseries calls |
tests/example-data/.../process_deploymentRealTime.py | Add deadreckon=False to example script invocation |
tests/example-data/.../deploymentRealtime.yml | Switch latitude/longitude source: keys to Lat /Lon |
pyglider/seaexplorer.py | Implement deadreckon logic for filtering lat/lon |
Comments suppressed due to low confidence (2)
tests/test_pyglider.py:100
- [nitpick] There’s no test covering the default
deadreckon=False
path—adding a case to confirm that underwater positions are set to NaN whendeadreckon
is not passed would help ensure correct behavior.
seaexplorer.raw_to_L0timeseries(
tests/test_pyglider.py:101
- Verify that
raw_to_L0timeseries
actually accepts adeadreckon
keyword argument; if not, either update that function or change this test to callraw_to_timeseries
directly.
rawncdir, l0tsdir, deploymentyaml_raw, kind='raw', deadreckon=True
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a deadreckon
option to raw_to_timeseries
that disables dead-reckoned positions and only uses valid surface fixes for lat/lon. It also updates tests, example scripts, and YAML to pass the new flag and adjust coordinate sources.
- Add
deadreckon
keyword (defaultFalse
) toraw_to_timeseries
and implement helper functions to drop and forward‐fill coordinates. - Update unit tests and example workflows to pass the
deadreckon
flag. - Change example deployment YAML to use
Lat
/Lon
sources instead ofNAV_LATITUDE
/NAV_LONGITUDE
.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
File | Description |
---|---|
tests/test_pyglider.py | Updated calls to include deadreckon=True in timeseries tests |
tests/example-data/.../process_deploymentRealTime.py | Pass deadreckon=False to raw_to_timeseries in example script |
tests/example-data/.../deploymentRealtime.yml | Switch lat/lon sources from NAV_LATITUDE /NAV_LONGITUDE to Lat /Lon |
pyglider/seaexplorer.py | Added deadreckon parameter, _forward_fill & _drop_if helpers, and coordinate‐dropping logic |
Comments suppressed due to low confidence (2)
tests/test_pyglider.py:101
- The test calls
raw_to_L0timeseries
withdeadreckon=True
, butraw_to_L0timeseries
may not accept that parameter. Either extend its signature to pass throughdeadreckon
or update the test to callraw_to_timeseries
directly.
outname = seaexplorer.raw_to_L0timeseries(rawncdir, l0tsdir, deploymentyaml_raw, kind='raw', deadreckon=True)
tests/test_pyglider.py:150
- Similarly, this call to
raw_to_L0timeseries
passesdeadreckon=True
but the function signature doesn't include it. Consider updating that function or test accordingly.
outname_interp_raw = seaexplorer.raw_to_L0timeseries(rawncdir, l0tsdir_interp_raw, interp_yaml, kind='raw', deadreckon=True)
|
||
|
||
def _drop_if(gli, todo='Lat', condit='DeadReckoning', value=1): | ||
"""Drop Lat if DeadReckoning is 1""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Docstring for _drop_if
is hardcoded to 'Lat' and 'DeadReckoning'; generalize to mention parameters todo
, condit
, and value
.
"""Drop Lat if DeadReckoning is 1""" | |
""" | |
Drop values in the column specified by `todo` if the column specified by `condit` equals `value`. | |
Parameters: | |
gli (polars.DataFrame): The input DataFrame. | |
todo (str): The name of the column to modify. | |
condit (str): The name of the column to check the condition against. | |
value (any): The value to compare against in the `condit` column. | |
Returns: | |
polars.DataFrame: The modified DataFrame with updated values in the `todo` column. | |
""" |
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <[email protected]>
@richardsc can you guys test this when you get a chance? |
Hi @jklymak -- sorry for being slow. @clayton33 and I were re-running some of our missions to see if this behaved as expected. It does! (though as a heads-up we will be working through some other issues we found, in the case where a user might actually want to keep and use some of these nav variables, such as deadReckoning, navState, etc). |
This closes #4
For
raw_to_timeseries
I've added a new kwargdeadreckon=False
which ignores the dead reckoning, and only has valid lat and Lon when the glider is on the surface.This is the default, but if you use
NAV_LATITUDE
andNAV_LONGITUDE
in the yaml, you probably want to change toLat
andLon
respectively to pick up the lat and Lon in the glider file.