Skip to content
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

NPI-3683 SP3 consistency check streamlining #70

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

treefern
Copy link
Collaborator

This PR adds various restructuring of the SP3 read and validation process, aimed at improving consistency and separating concerns better between stages of the process.

Also:

  • Stopped setting header ORB_TYPE to INT when removing offline sats.
  • Added utility functions to derive unique epochs and SVs, to capture the nuances in a single place.
  • Added remove_svs_from_header() utility function for removing sats from internal header, so we can better maintain header consistency when performing operations like remove_offline_sats()
  • Improved filter_by_svs(), reworking the removal logic to only call drop() once, and to update the internal header to reflect the new list of sats.
  • Added experimental detection and warnings for old / new versions of SP3 format.

…eader representation, to help maintain consistency of that header data. This is useful for example when removing offline sats.
…up by header parser, including a warning for old versions of the format which we may not have tested for
…in the list of SVs - only impact in validation code because things were not well ordered
…ion of SP3 header when it removes an SV from the dataframe
…nd various check and clean-up functions, to streamline code and separate concerns. The dataframe should now be more streamlined by the time checks are invoked, meaning the checks have less edge cases to consider.
…new list of SVs. Filtering logic reworked to maintain a unified set of SVs to keep, converting this into a set to drop just before applying. This should avoid the potential to end up with filter_by_name and filter_by_count selecting different SVs and ending up with an empty output. test_file_creation_util.py updated to allow epoch count to remain unchanged, and to use new validation logic when reading files.
@treefern treefern requested a review from ronaldmaj January 13, 2025 05:09
@treefern treefern self-assigned this Jan 13, 2025
Copy link
Collaborator

@ronaldmaj ronaldmaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A nice general clean-up to the sp3 reading / editing functionality, plus relevant unit-tests.
I've run the tests locally, all seems to work well. I don't see any major issues, just one type-hint / docstring to address. Other than that I think it's good to go

@@ -193,14 +196,41 @@ def sp3_clock_nodata_to_nan(sp3_df: _pd.DataFrame) -> None:
sp3_df.loc[nan_mask, ("EST", "CLK")] = _np.nan


def remove_svs_from_header(sp3_df: _pd.DataFrame, sats_to_remove: set[str]):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing output type hint

DataFrame. This is useful e.g. when removing offline satellites.
:param _pd.DataFrame sp3_df: SP3 DataFrame on which to update the header (in place).
:param list[str] sats_to_remove: list of SV names to remove from the header.
"""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doc string missing output parameter

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is none; this function operates on the DataFrame passed in, modifying it in place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants