-
Notifications
You must be signed in to change notification settings - Fork 9
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
Fix clock normalisation and convert sp3 position nodata values to NaNs #30
Conversation
EugeneDu-GA
commented
Jul 11, 2024
- Fixed 'daily' and 'epoch' clock normalisation with compare_clk() when one of the files have missing data;
- Added option to convert sp3 positional nodata values to NaNs.
Awesome, thanks for the PR @EugeneDu-GA ! |
Yes I have, where to upload them? |
If they're from CDDIS or elsewhere, perhaps just the link to the files, otherwise it seems like you can attached files to comments in GitHub so you should be able to put them here |
Test examples & results: Test 1.
After Fixing:
Note that orbq automatically removes satellites containing NaN values. 2. Example files: |
Backlog: option to keep valid orbital data/epoch of a satellite containing NaN values. |
This is a good idea. You can probably write this up as a ticket so we have it for future reference |
Awesome, thanks so much for putting this together and showing the improvement with before / after examples. I've run this locally and I'm getting the same results as you, so seems to be working well. I've added a couple more comments in individual lines of code, but I think this is mostly all good to merge in |
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.
This is great, I think this has come out really nicely and will make comparisons via the CLI easier.
The one change I've requested is just to use the one-line-sphinx
documentation style for the new function you've introduced. Everything else looks good!
gnssanalysis/gn_io/sp3.py
Outdated
SP3_CLOCK_STD_NODATA = -1000 | ||
SP3_POS_STD_NODATA = -100 | ||
|
||
|
||
def sp3_pos_nodata_to_nan(sp3_df): |
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.
It's a long term clean-up project, but we're hoping to slowly move all python code to follow a documentation standard. In our case, we've chosen one-line sphinx style: https://github.com/NilsJPWerner/autoDocstring/blob/HEAD/docs/one-line-sphinx.md
There's an example of this in the compare_clk function:
gnssanalysis/gnssanalysis/gn_diffaux.py
Line 307 in 2297ba7
def compare_clk( |
Ideally all new functions follow this format and in future we'll work towards going through the backlog and changing all the old functions to the new doc standard as well
I think the main thing for your function sp3_pos_nodata_to_nan()
is to include type hints and an explanation of the input / outputs in the Docstring
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.
Great! Thanks for getting those docstrings put through. Ready to go in now 👍