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

Update the one-sentence description of wrappers to match upstream documentation #3857

Merged
merged 5 commits into from
Mar 21, 2025

Conversation

seisman
Copy link
Member

@seisman seisman commented Mar 18, 2025

@seisman seisman added documentation Improvements or additions to documentation skip-changelog Skip adding Pull Request to changelog needs review This PR has higher priority and needs review. labels Mar 18, 2025
@seisman seisman added this to the 0.15.0 milestone Mar 18, 2025
@@ -1,5 +1,5 @@
"""
solar - Plot day-night terminators and twilight.
solar - Plot day-light terminators and other sunlight parameters.
Copy link
Member Author

Choose a reason for hiding this comment

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

The upstream documentation uses "day-light terminators" (https://docs.generic-mapping-tools.org/latest/solar.html), but sometimes it also uses "day-night" terminators.

It seems "day-night" is correct and there is an upstream typo, right?

Copy link
Member

Choose a reason for hiding this comment

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

Should be day-night in my opinion, day-light does not make sense to me.

Copy link
Member

Choose a reason for hiding this comment

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

I agree that the formulations on this in the GMT and PyGMT docs and examples are currently inconsistent and definitely confusing.

I feel that the idea was/ is that "day-light terminators" is the summary of all four options for the terminator parameter, i.e. day-night terminator as well as the civil, nautical and astronomical twilights.

Copy link
Member Author

@seisman seisman Mar 18, 2025

Choose a reason for hiding this comment

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

"day-light terminators" is the summary of all four options for the terminator parameter, i.e. day-night terminator as well as the civil, nautical and astronomical twilights.

It makes sense to me, so I think the GMT documentation is correct, right?

Copy link
Member

Choose a reason for hiding this comment

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

Hm. I think it is not directly wrong, but I do not know if this is an official formulation. And I am ab it wondering if this is directly clear for users. At least I needed some time to get it.

BTW we should also be consistently use either day-night terminator or day/night terminator.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm. I think it is not directly wrong, but I do not know if this is an official formulation. And I am ab it wondering if this is directly clear for users. At least I needed some time to get it.

The term has been in the GMT documentation for many years and no one complains about it, so maybe it's the correct term.

BTW we should also be consistently use either day-night terminator or day/night terminator.

"day/night terminator" is not used anywhere. Am I missing something?

Copy link
Member

Choose a reason for hiding this comment

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

In line 52 we use currently "day_night": Day/night terminator

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@michaelgrund michaelgrund added final review call This PR requires final review and approval from a second reviewer and removed needs review This PR has higher priority and needs review. labels Mar 20, 2025
@@ -1,5 +1,5 @@
"""
solar - Plot day-night terminators and twilight.
solar - Plot day-light terminators and other sunlight parameters.
Copy link
Member

Choose a reason for hiding this comment

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

In line 52 we use currently "day_night": Day/night terminator

Comment on lines +29 to 31
Plot base maps and frames.
Creates a basic or fancy basemap with axes, fill, and titles. Several
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should consistently use either "basemap" or "base map".

@seisman seisman merged commit fdfe687 into main Mar 21, 2025
23 checks passed
@seisman seisman deleted the doc/module-description branch March 21, 2025 13:07
@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Mar 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation skip-changelog Skip adding Pull Request to changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants