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-3685 Streamline SP3 incorrect timerange unit test #71

Merged
merged 8 commits into from
Jan 31, 2025

Conversation

treefern
Copy link
Collaborator

This PR attempts to speed up one of our slower unit tests, which parses a full day worth of SP3 data.

The number of sats in the test file has been dropped from 30 to 1.
Test run time is still slow, but perhaps 1-2 seconds faster than it was.

After a bit of investigation, it appears the main bottleneck in reading SP3 files, is the fact each epoch is parsed into a temporary DataFrame, before being concatenated into the output DataFrame. So, without significantly overhauling the relevant code, number of epochs is likely to be the main determiner of speed.

Also added:

  • a small and experimental utility class ContextTimer, which can be used as a context manager, to time function executions.
  • use of pyfakefs reset() in various unit tests, to better guard against data from previous test runs impacting tests.

@treefern treefern requested a review from ronaldmaj January 13, 2025 05:28
@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.

This looks good to me - nothing major to comment on. The addition of the reset() method is great to lower chances of issues with our unit tests in future

@treefern treefern merged commit ecdde74 into main Jan 31, 2025
4 checks passed
@treefern treefern deleted the NPI-3685-streamline-sp3-incorrect-timerange-unit-test branch January 31, 2025 08:02
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