Skip to content

Conversation

seisman
Copy link
Member

@seisman seisman commented Sep 18, 2025

Some wrappers have the no_clip parameter, which can be either False or True. This PR migrates the parameter to the new alias system.

no_clip in Figure.plot and Figure.plot3d are not updated in this PR, because in these two methods, no_clip can be False, True, "c" or "r" and we need to discuss the long-form arguments for "c"/"r". So leave it to a separate PR instead.

@seisman seisman added this to the 0.17.0 milestone Sep 18, 2025
@seisman seisman added maintenance Boring but important stuff for the core devs skip-changelog Skip adding Pull Request to changelog needs review This PR has higher priority and needs review. labels Sep 18, 2025
Comment on lines +113 to +115
no_clip
Do **not** clip contours or image at the frame boundaries [Default is ``False``
to fit inside ``region``].
Copy link
Member

Choose a reason for hiding this comment

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

I am wondering to what "image" is refering here.

Copy link
Member Author

Choose a reason for hiding this comment

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

It comes from the upstream documentation at https://docs.generic-mapping-tools.org/6.5/contour.html#n.

Here "image" is used because contour can color the triangles using a CPT. See the example below:

gmt begin map
    gmt makecpt -Cseis -T600/1000/100
    gmt basemap -R0.5/6/0.5/6 -JX10c/10c -Baf
    gmt plot @Table_5_11.txt -Sc0.2c
    gmt contour @Table_5_11.txt -Wthin -C -I -N -t50
gmt end show
map

Comment on lines 78 to 82
- J = projection
- N = no_clip
- V = verbose
- c = panel
- t = transparency
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- J = projection
- N = no_clip
- V = verbose
- c = panel
- t = transparency
- J = projection
- N = no_clip
- V = verbose
- c = panel
- t = transparency

Copy link
Member

Choose a reason for hiding this comment

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

Hm, not sure what GitHub is doing here, but actually I only removed one whitespace before the newly added bullet point for -N.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why do you want to remove one whitespace before -N? I believe the current one is correct. See https://pygmt-dev--4103.org.readthedocs.build/en/4103/api/generated/pygmt.Figure.grdimage.html#pygmt.Figure.grdimage

Copy link
Member

Choose a reason for hiding this comment

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

Actually, I am unsure. Directly on GitHub it looks like all bullet points have the same number of leading whitespaces, but when I look at it in the suggestion view, it seems like there is one whitespace more for -N:

github suggestion view
list_alias_whitspace list_alias_whitspace_review

Copy link
Member Author

Choose a reason for hiding this comment

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

Same for me with the new GitHub UX. It's more likely a GitHub bug.

Copy link
Member

@yvonnefroehlich yvonnefroehlich Sep 24, 2025

Choose a reason for hiding this comment

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

Yeah, I experienced some unexpected behavior in the last days... Then we just leave this as is and in case one of the lists is not correctly aligned, we can still make a PR to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Boring but important stuff for the core devs 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.

2 participants