Skip to content

DOC: Improve link to the GMT documentation #3944

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

seisman
Copy link
Member

@seisman seisman commented May 10, 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 May 10, 2025
@seisman
Copy link
Member Author

seisman commented May 10, 2025

I'll apply the same changes to other wrappers if approved.

Copy link
Member

@yvonnefroehlich yvonnefroehlich left a comment

Choose a reason for hiding this comment

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

For me this change looks good in the docs.

@seisman
Copy link
Member Author

seisman commented May 11, 2025

Ping @michaelgrund and @weiji14 for comments before I make changes.

Actually, I'd still like to rephrase the sentence so the links appear on one line, but I haven’t found a good solution yet.

@seisman seisman added this to the 0.16.0 milestone May 12, 2025
Comment on lines 128 to 132
.. note::

Wraps the GMT module ``grdfill``.
The GMT documentation is at :gmt-docs:`grdfill.html`.

Copy link
Member

Choose a reason for hiding this comment

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

Hm. If it should fit into one line, maye we can put the link to the docs within the grdfill.
Not sure, if the link works in this way or the full URL is required.

Suggested change
.. note::
Wraps the GMT module ``grdfill``.
The GMT documentation is at :gmt-docs:`grdfill.html`.
.. note::
Wraps the GMT module [``grdfill``](:gmt-docs:`grdfill.html`).

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure if your solution works, but the one below is shorter and should work. This way, the URL isn't explicitly shown, so readers might not realize that it links to the GMT documentation. Is it OK?

Suggested change
.. note::
Wraps the GMT module ``grdfill``.
The GMT documentation is at :gmt-docs:`grdfill.html`.
Wraps the GMT module :gmt-docs:`grdfill <grdfill.html>`.

Read a grid that presumably has unfilled holes that the user wants to fill in some
fashion. Holes are identified by NaN values but this criteria can be changed via the
``hole`` parameter. There are several different algorithms that can be used to
replace the hole values. If no holes are found the original unchanged grid is
returned.

Full option list at :gmt-docs:`grdfill.html`.
Copy link
Member

Choose a reason for hiding this comment

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

The note:: admonition stands out a bit too much. Perhaps just reword Full option list at ... to Full GMT docs at ..., similar to @ezevazquez's suggestion at #3881 (comment)?

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 note:: admonition stands out a bit too much.

I agree.

Perhaps just reword Full option list at ... to Full GMT docs at ...

I prefer to explicitly mention the name of the GMT module, since the function/method names may differ from the module names. What about the one below?

image

Copy link
Member

Choose a reason for hiding this comment

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

since the function/method names may differ from the module names

I thought we usually use the same method name as upstream GMT? Different ones I can find are:

The names seem to follow closely enough that we don't need to repeat the name really.

Copy link
Member Author

@seisman seisman May 14, 2025

Choose a reason for hiding this comment

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

We have methods like hlines/vlines and will have methods like scatter (#3602 ) and choropleth (#2798), which wrap plot. I guess we still need to link to plot in these methods.

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 needs review This PR has higher priority and needs review. skip-changelog Skip adding Pull Request to changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants