Fix Calculation of GM in `osculating_elements_of()`, etc.

skyfielders / python-skyfield

Elegant astronomy for Python

MIT License

1.4k stars 211 forks source link

Fix Calculation of GM in `osculating_elements_of()`, etc. #664

Closed JoshPaterson closed 2 years ago

JoshPaterson commented 2 years ago

fixes calculation of GM in osculating_elements_of()
Adds calculated value for solar system barycenter to skyfield.data.gravitational_parameters.GM_dict. This is calculated by summing all of the masses in GM_dict that aren't barycenters.
Add keyword argument to osculating_elements_of(). I named this argument gm_km3_s2 to match the arguments @brandon-rhodes used in data.mpc.py. Most places in skyfield use gm_km3_s2 as the argument name and I think that's the best and most explicit option. I believe OsculatingElements.__init__() is the last place in the public api using 'mu' in a parameter name, is there a way to deprecate this without breaking backwards compatibility?
Make the doc string for osculating_elements_of more informative

JoshPaterson commented 2 years ago

@brandon-rhodes can you point me to a skyfield object whose docstring format is a good model to follow? I'm having trouble getting the doctest to pass. Thanks!

brandon-rhodes commented 2 years ago

I believe OsculatingElements.init() is the last place in the public api using 'mu' in a parameter name, is there a way to deprecate this without breaking backwards compatibility?

Alas, no, I think that's now fixed. But maybe we can deprecate that class at some point? I remember our discussing whether there should be a reworking of the way that orbits relate to elements, but I don't remember what we were thinking of doing. I should return to that conversation soon when I have time.

JoshPaterson commented 2 years ago

Oh yeah, that's right, we did discuss deprecating the OsculatingElements object in #481, hopefully I will have some time to work on that this winter!

Also, thanks for the detailed feedback, I learn a lot from you when I contribute to Skyfield, which is part of why I enjoy doing so!

brandon-rhodes commented 2 years ago

Also, thanks for the detailed feedback, I learn a lot from you when I contribute to Skyfield, which is part of why I enjoy doing so!

Well, thank you for being such a good sport about it :)

[Edit: removed paragraph in which I expressed confusion about why the body's own mass isn't needed but only the sum of the two masses; after more thought, I've answered my own question.]