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

Add conversion from geodetic to geocentric spherical #54

Closed
leouieda opened this issue Jun 15, 2022 · 4 comments · Fixed by #55
Closed

Add conversion from geodetic to geocentric spherical #54

leouieda opened this issue Jun 15, 2022 · 4 comments · Fixed by #55

Comments

@leouieda
Copy link
Contributor

There is a function for converting latitudes from geodetic to geocentric but this is restricted to the surface of the ellipsoid. For many geophysical applications, we need to convert the (latitude, height) of points into (latitude_spherical, radius).

This is implemented in Boule as boule.Ellipsoid.spherical_to_geodetic. Boule is more focused on gravity calculations than coordinate conversions so I'd be happy to move that implementation to pymap3d if it's of interest to you.

I can volunteer to do this given a bit of guidance as to where to put the functions and what they should be called to fit with pymap3d.

@scivision
Copy link
Member

scivision commented Jun 16, 2022

We can make it a separate file under src/pymap3d/, and then import it from src/pymap3d/__init__.py like the others.
Maybe src/pymap3d/spherical.py ?
The function naming comes from Matlab/Octave, which I was replacing with Python when I made pymap3d years ago. Following that arbitrary naming scheme, perhaps spherical2geoetic() ?

@leouieda
Copy link
Contributor Author

👍🏾 sounds good to me! I'll give this a shot and open a PR soon.

@ryanpavlick
Copy link
Contributor

ryanpavlick commented Jun 16, 2022 via email

@leouieda
Copy link
Contributor Author

@ryanpavlick thanks for the pointer, I missed that when going over the latitude module. It takes into account but doesn't calculate the geocentric radius coordinate. From the docstrings it seemed like this was just for latitude so I didn't think to look further.

I already added the 2 functions to #55 but could change it to patch the existing geocentric2geodetic functions to return radius as well. But this would either break backwards compatibility or required another if flag then return block. I'm personally not fond of those since it the documentation becomes more confusing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants