-
Notifications
You must be signed in to change notification settings - Fork 79
Extend OrbitElements Class with New Methods and Add Pytest Coverage
#204
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
Conversation
6422ebc to
a5a2791
Compare
djhoese
left a comment
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! This looks pretty good. I have one important question and one small request.
cdef8d4 to
51fa3ac
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #204 +/- ##
==========================================
+ Coverage 89.78% 90.43% +0.65%
==========================================
Files 17 18 +1
Lines 2809 3011 +202
==========================================
+ Hits 2522 2723 +201
- Misses 287 288 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
👀 oh I like the 0.8% increase in test coverage. Nice. |
djhoese
left a comment
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.
A couple more requests for the test code. Thanks again for working on this.
51fa3ac to
5d31a07
Compare
5d31a07 to
5d780e6
Compare
djhoese
left a comment
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.
pnuu
left a comment
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.
Thank you for the new features and the bug fix!
I've added one refactoring suggestion, but I'm not sure it really helps the code 😅
mraspaud
left a comment
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 a lot for your contribution! I'm really grateful for the nice cleanups and refactors along with a bug fix on top of the added functionality.
I have a minor improvement suggestion inline, but this looks ready otherwise I think.
Co-authored-by: Panu Lahtinen <pnuu+git@iki.fi>
827f749 to
ee88a15
Compare
3f89c57 to
e19dfc1
Compare
e19dfc1 to
9f800e0
Compare
9f800e0 to
b10cfeb
Compare
mraspaud
left a comment
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.
LGTM, thanks a lot for taking the time to improve pyorbital!
PR improves the
OrbitElementsclass by adding several new methods and properties to make it more functional and easier to use. It also introduces a full pytest test suite to verify the correctness of the class (before and after) and its behavior under different conditions.The changes include:
apogee,is_circular, andis_retrogradeposition_vector(),velocity_at_perigee(),velocity_at_apogee()andto_tle_dict()These additions were made as part of a satellite tracking project where I needed access to derived orbital parameters and a reliable way to validate them. The test suite ensures the class behaves correctly and can be extended in future work.