-
Notifications
You must be signed in to change notification settings - Fork 1
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
ENH/DOC: geocentric / geodetic info for sgp4 instrument, metadata improvement #66
Conversation
Putting this up as a draft pull into #60 to show downstream changes. Currently includes lat / long / alt calculations from both spherical and ellipsoidal assumptions for comparison and testing. |
Marking this as ready for review, but testing of expected orbits is required. Please try out whatever options you can think of when generating orbits. |
not sure if this is because of the way I installed or something, but I only get data when I load dates between 2017-2019 |
It's tied to how we inherit the instrument definition from the core code. Because a file has to exist, we have a fake routine for list files. We should be able to fix this at the instrument level by passing through a wider range of dates here:
Replacing the above with
should allow a wider range of dates. |
Conversely, you can set a new range when initializing without changing the code:
I keep forgetting about this feature. |
Back on my awareness list! Will take a look tomorrow. Apologies for the delay. |
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.
Thanks for the improvements @jklenzing!
bstar=None, num_samples=None, cadence='1S'): | ||
def load(fnames, tag=None, inst_id=None, TLE1=None, TLE2=None, | ||
alt_periapsis=None, alt_apoapsis=None, | ||
inclination=None, raan=0., arg_periapsis=0., mean_anomaly=0., |
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.
I see that raan and arg_periapsis etc. are used based on kwarg settings. Seems possible a user may not know when they are used. I'd recommend adding info on the docstring to inclination that it is the key for the other parameters. Info for the other variables should be updated to reflect the new defaults.
Thanks @rstoneback. I've updated docstrings and added a test for one of the new options. |
Description
Addresses #56, #55, #16
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
By loading satellite data and plotting orbits. Example load:
The output should match expectations, with max latitude corresponding to inclination, and altitude range described by periapsis and apoapsis. Note that both spherical (eg, latitude) and elliposidal(eg, geod_latitude) calculations are included. There remains some discrepancy on the order of 10 km in expectations. This could be due to the
methods.convert_from_keplerian
method.Test Configuration:
Checklist:
develop
(notmain
) branchCHANGELOG.md
, summarizing the changes