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

Docs: Add new Ellipsoids page to explain ellipsoidal parameters. #2922

Merged
merged 20 commits into from
Nov 20, 2021

Conversation

direvus
Copy link
Contributor

@direvus direvus commented Nov 5, 2021

This PR is a continuation of #2385, which appears to have been abandoned by its original author.

I haved merged the commits from that PR, incorporated the PR feedback, and made a few changes and improvements of my own.

I hope you find this helpful.

RohitPingale and others added 17 commits October 13, 2020 16:24
fixing the spaces indentation and small typo.
Pull in commits by @RohitPingale, originally submitted under PR OSGeo#2385.
That PR was closed due to inactivity. My intention is to pick up the
work where they left off.
Incorporate suggested changes from @kbevers on the original PR OSGeo#2385.
The author the original figure did not provide a source for the image.
The image also had "geoid" misspelled in its labels, and label placement
was somewhat confusing.

This new image was created from scratch to match the design intent of
the original image, but reuses none of its assets directly.  I include
the SVG source.  This image is licensed CC0.
docs/source/news.rst Outdated Show resolved Hide resolved
@kbevers
Copy link
Member

kbevers commented Nov 5, 2021

Thanks for taking this up again. In terms of the commit history this PR is a bit messy, so I'll just squash merge it when the time comes. That will make the two of you co-authors of that single commit, which I guess reflect the state of things quite well.

Copy link
Member

@mwtoews mwtoews left a comment

Choose a reason for hiding this comment

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

Hope you don't mind a commit here to update a few other things, including to indent the option and use math formatting. The comment in the code for "R_V" needed to be corrected, as it was an old typo.

docs/images/general_ellipsoid.svg Show resolved Hide resolved
@direvus
Copy link
Contributor Author

direvus commented Nov 10, 2021

Thanks for the additional markup improvements!

Good pickup on the "semi" axis labels too, I feel a bit silly for not noticing that. Guess I was too focussed on the diagram design and not thinking critically about its contents.

I think splitting those axes in half and tuning down the opacity on the unlabelled half will do the trick. I'll make that change on the branch.

Divide the axis lines into two parts and fade out the unlabelled part,
to indicate that "semi-" refers to one half of the axis only.
Copy link
Member

@mwtoews mwtoews left a comment

Choose a reason for hiding this comment

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

Just one missed formula, and undo the edit to docs/source/news.rst (suggested by @kbevers) and this will be nice to merge. Thanks for getting this one finished!

docs/source/usage/ellipsoids.rst Outdated Show resolved Hide resolved
Add math markup to "1/f" expression.

These changes per PR feedback from @mwtoews and @kbevers.
@direvus direvus force-pushed the 1064-docs-ellipsoid branch from 8d0ec2f to 3fba3df Compare November 19, 2021 23:34
@direvus
Copy link
Contributor Author

direvus commented Nov 19, 2021

Thanks @mwtoews, I think I've got those final changes covered now.

@mwtoews mwtoews merged commit 5bb20f7 into OSGeo:master Nov 20, 2021
github-actions bot pushed a commit that referenced this pull request Nov 20, 2021
Co-authored-by: Rohit <rohitpingale103@gmail.com>
Co-authored-by: Brendan Jurd <brendan.jurd@geoplex.com.au>
Co-authored-by: Mike Taves <mwtoews@gmail.com>
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.

Document ellipsoid parameters
4 participants