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 geodetic to geocentric coordinates conversion #28

Merged
merged 21 commits into from
Nov 16, 2018

Conversation

santisoler
Copy link
Member

Convert geocentric coordinates (defined by the ReferenceEllipsoid) to
geocentric (spherical) coordinates. Only geodetic latitude and height
are converted to geocentric_latitude and radius, geocentric and
geodetic latitudes are equal. Add tests for new function: (1) test with
a "spherical" ellipsoid (in such case the geodetic and gecentric
coordinates are the same) (2) test with real ellipsoids on equator and
poles (on those points geodetic and geocentric latitudes are must be
equal).

Fixes #20

Reminders

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst.
  • Write detailed docstrings for all functions/methods.
  • If adding new functionality, add an example to the docstring, gallery, and/or tutorials.

@santisoler
Copy link
Member Author

@leouieda
The CI will fail because ReferenceEllipsoid does not have first_eccentricty among its attributes on this branch.
We should merge #27 before this.

@leouieda
Copy link
Member

leouieda commented Nov 9, 2018

👍 Merged #27 so feel free to update/rebase this branch.

Copy link
Member

@leouieda leouieda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@santis19 just a minor comment but something we should think about. We can also merge this and change the name later but that's usually more work.

harmonica/coordinates.py Outdated Show resolved Hide resolved
harmonica/coordinates.py Outdated Show resolved Hide resolved
# To do so, we define a zero flattening, thus an infinite inverse flattening.
spherical_ellipsoid = ReferenceEllipsoid(
"unit_sphere", sphere_radius, np.infty, 0, 0
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I'm increasingly happy with the ellipsoid mechanics in Harmonica.

@leouieda
Copy link
Member

I merged in the changes from master so please remember to git pull

@leouieda
Copy link
Member

@santis19 this looks good to me.

Just thinking if we should standardize the API for things that take in coordinates. It might be good to always have (longitude, latitude, height) so that we make calls like geodetic_to_spherical(*coordinates) instead of geodetic_to_spherical(coordinates[1], coordinates[2]). What do think?

@leouieda
Copy link
Member

I'm going to merge this and we'll think about the arguments a bit more before making changes.

Thanks @santis19

@leouieda leouieda merged commit ebd6ebc into fatiando:master Nov 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Function to transform geodetic to geocentric spherical coordinates
2 participants