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-3676 Exception handling on (unsupported) SP3 velocity data output #66

Merged
merged 4 commits into from
Jan 8, 2025

Conversation

treefern
Copy link
Collaborator

@treefern treefern commented Jan 7, 2025

gnssanalysis currently does not correctly support output of SP3 velocity data.

At present, currently calling gen_sp3_content() on a DataFrame containing velocity data, will result in a non-compliant SP3 file with velocity data in additional columns (not in additional, interlaced rows as the spec calls for).

This PR adds a check for velocity columns. If any velocity columns are found a warning is logged, the columns are deleted, then output continues as normal on just the position data.

Optionally based on a new parameter to gen_sp3_content(), a NotImplementedError can be raised instead of logging a warning.

@treefern treefern requested a review from ronaldmaj January 7, 2025 08:51
@treefern treefern self-assigned this Jan 7, 2025
@treefern
Copy link
Collaborator Author

treefern commented Jan 7, 2025

A small fix for the sp3_merge() unit test came along for the ride.
It turns out that pyfakefs actually uses a real temporary directory under the hood. It seems this can lead to data persisting between unit test runs!

ronaldmaj
ronaldmaj previously approved these changes Jan 8, 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.

This is a nice clean-up PR and sets up a new unit-test for future. Happy for this to go in

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.

All good from what I can see after the merge with main - good to go

@ronaldmaj ronaldmaj merged commit 36c0374 into main Jan 8, 2025
4 checks passed
@treefern treefern deleted the NPI-3676-warn-on-sp3-velocity-output branch January 8, 2025 04:21
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