fatiando / boule

Reference ellipsoids for geodesy and geophysics
https://www.fatiando.org/boule
BSD 3-Clause "New" or "Revised" License
38 stars 17 forks source link

Add geocentric_radius to TriaxialEllipsoid #146

Closed blazej-bucha closed 1 year ago

blazej-bucha commented 1 year ago

Add a new geocentric_radius method to TriaxialEllipsoid that computes the geocentric surface radius of points in triaxial ellipsoids. Add two new flattening properties to the class: the meridional_flattening and the equatorial_flattening. Add tests and references for the equations used.


Please take a look at the PR to check if it makes sense.

Relevant issues/PRs:

Fixes #105

welcome[bot] commented 1 year ago

💖 Thank you for opening your first pull request in this repository! 💖

A few things to keep in mind:

No matter what, we are really grateful that you put in the effort to do this!

santisoler commented 1 year ago

Hi @blazej-bucha! Thanks so much for opening this PR! I'm really sorry for the delay of my reply.

I think this is a great addition. Let me review it a little bit and will come back with comments.

Regarding the code-style Action that fails, apparently you run an older version of Black that used to format every power operator (**) with leading and trailing spaces. The updated version of Black we use in Boule formats simple power operators without spaces between the base and the power (check this section in Black's docs).

blazej-bucha commented 1 year ago

Hi @santisoler,

Thanks for looking at the pull request and for your comments! All your suggestions should now be applied. In addition, reverting the order of input parameters in geocentric_radius necessitated a few more edits in tests that call this method.

Formating should now hopefully be OK. Previously, I used indeed an older version of Black from Debian repos.

blazej-bucha commented 1 year ago

@santisoler, thanks for your edits! I directly commited all of them.

I'm now happy with the pull request, too. Thanks again for reviewing the code.

santisoler commented 1 year ago

Thanks @blazej-bucha for the contribution! I've just made a few style changes and removed some unused variables that flake was complaining about. I'm merging this! 🚀

welcome[bot] commented 1 year ago

🎉 Congrats on merging your first pull request and welcome to the team! 🎉

If you would like to be added as a author on the Zenodo archive of the next release, add your full name, affiliation, and ORCID (optional) to the AUTHORS.md file of this repository. Feel free to do this in a new pull request if needed.

We hope that this was a good experience for you. Let us know if there is any way that the contributing process could be improved.

leouieda commented 1 year ago

@blazej-bucha thanks for this! Please see the message above ☝🏾 about adding yourself to the AUTHORS.md file.