Turn longitude_semimajor_axis into an optional attribute #183
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MarkWieczorek
force-pushed
the
triaxial_centrifugal_potential
branch
from
dfc7a72
to
fc8ca61
Compare
…antiate the instance
MarkWieczorek
force-pushed
the
triaxial_centrifugal_potential
branch
from
ca20bb7
to
a7c6470
Compare
VascoSch92
reviewed
Apr 8, 2024
Merged
|
This PR was incorporated into #180 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR turns
longitude_semimajor_axis(renamed tosemimajor_axis_longitude) into an optional attribute that is used to instantiate triaxial ellipsoids.Previously, this parameter was found as an optional argument to the
geocentric radiusfunction. However, this parameter is an intrinsic property of the ellipsoid, and will be necessary for all routines that make use of longitude coordinates. As this is now declared as a class attribute, I have thus remove this optional parameter fromgeocentric radius.The
test_geocentric_radius_semimajor_axis_longitudewas somewhat modified: I declared a new ellipsoidtriaxialellipsoid_90that setsemimajor_axis_longitudeto 90.As soon as this PR is merged, I will add the
centrifugal_potentialmethod (which requires the updatedgeocentric_radiusfunction).Note that at some point, we may want to allow for ellipsoids that are not rotating about the z axis, but this is probably far off into the future.
Relevant issues/PRs: