Figure.grdcontour: Adjust processing of arguments passed to the "annotation" and "interval" parameters, deprecate "sequence_plus"

[yvonnefroehlich]

Description of proposed changes

This PR aims to adjust the arguments passed to the annotation and interval parameters of Figure.grdcontour:

• input options
  - a fixed interval als float 100 -> 100
  - one level as list: [100] -> 100, (mention the trailing comma)
  - multiple levels as list: [100, 200] -> 100,200
  - no as string: "n" -> n (syntax of GMT 6.5)
• Deprecated sequence_plus for annotation instead use one string, e.g., [100, "f10p", "gred"], now "100+f10p+gred"
• Update docstrings
• Add tests for one level and multiple levels

Related:

• GMT docs
  • annotation: https://docs.generic-mapping-tools.org/dev/grdcontour.html#a
  • interval: https://docs.generic-mapping-tools.org/dev/grdcontour.html#c
• PR for Figure.contour: Figure.contour: Adjust processing of arguments passed to the "annotation" and "levels" parameters #2706

Preview: https://pygmt-dev--3116.org.readthedocs.build/en/3116/api/generated/pygmt.Figure.grdcontour.html

Reminders

• Run make format and make check to make sure the code follows the style guide.
• Add tests for new features or tests that would have caught the bug that you're fixing.
• Add new public functions/methods/classes to doc/api/index.rst.
• Write detailed docstrings for all functions/methods.
• If wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs.
• If adding new functionality, add an example to docstrings or tutorials.
• Use underscores (not hyphens) in names of Python files and directories.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash command is:

• /format: automatically format and lint the code

[yvonnefroehlich]

I am wondering whether here the squared brackets are used to indicate that annot_int should be replaced with the desired value by the user. I find this a bit misleading.

The rounded brackets should be replaced by squared brackets as we write "list of arguments".

[seisman]

I don't think the description is correct. In the PyGMT world, a list of arguments means the option is repeatable. For example frame=["WSen", "xaf", "yaf"] is translated to -BWSen -Bxaf -Byaf. For this parameter, it must be annot_int+e+f10+gred, not a list.

[yvonnefroehlich]

Hm. I tried the following code to understand the input expected by annotation. It works for passing a list and I get the same output as for passing a string.

# orientated on https://www.pygmt.org/dev/api/generated/pygmt.Figure.grdcontour.html
# last access: 2024/03/18

import pygmt

region = [-92.5, -82.5, -3, 7]

grid = pygmt.datasets.load_earth_relief(resolution="15m", region=region)

fig = pygmt.Figure()
fig.grdcontour(
    projection="M10c",
    region=region,
    grid=grid,
    # annotation="500+f10p+gred",  # string -> WORKS
    # annotation=([500], "f10p", "gred"),  # -> FAILS
    # annotation=(500, "f10p", "gred"),  # -> WORKS
    annotation=[500, "f10p", "gred"],  # list -> WORKS
    frame=True,
)
fig.show()

# fig.savefig(fname="grdcontour_A_string.png")
# fig.savefig(fname="grdcontour_A_list.png")

Output figure

[seisman]

pygmt/pygmt/src/grdcontour.py

Lines 30 to 32 in 6fd4a00

	@kwargs_to_strings(
	R="sequence", L="sequence", A="sequence_plus", c="sequence_comma", p="sequence"
	)

I see. It's because A is defined like above, so the list of arguments is joined by +.

Actually, this is the only parameter that sets to sequence_plus.

[seisman]

I think we should disallow the syntax like annotation=[500, "f10p", "gred"], because:

1. From a user's perspective, it's no different from passing a string like 500+f10p+gred
2. The syntax is not well documented and is only used for this single parameter
3. It's not Pythonic at all, and it's very likely that we won't support such syntax in the new alias system (POC: WIP: New alias system #2949 but still WIP)

So, I think we should remove A="sequence_plus" and update the documentation.

[yvonnefroehlich]

I was actually also a bit wondering if we need this possibility, as such a list does not improve the readability significantly compared to the corresponding string. So, I think I am fine with removing A"sequence_plus".

[seisman]

contours is annotation interval in data units; it is ignored if contour levels are given in a file via -C. [Default is no annotations]. Prepend n to disable all annotations implied by -C. To just select a few specific contours give them as a comma-separated string; if only a single contour please add a trailing comma so it is seen as a list and not a contour interval.

So, valid arguments are for the -C option are:

• 100 means every 100 contours
• 100, means a single specific contour with value=100
• 100,200,300 means multiple specific contours with value=100, 200 and 300
• n means disable all contours

The corresponding Pythonic arguments are:

• a float value: annotation=100
• a list with a single value: annotation=[100]
• a list with multiple values: annotation=[100, 200, 300]
• a string: annotation="n"

I think we should change A="sequence_plus" to A="sequence" instead, to accept arguments like annotation=[100, 200, 300].

[yvonnefroehlich]

Actually there is already PR #2706 which is about updating / improving annotation and the levels or interval parameters of Figure.contour and Figure.grdcontour. Probably it makes sense to bundle this change in one of these PRs and close the other one.

[seisman]

Actually there is already PR #2706 which is about updating / improving annotation and the levels or interval parameters of Figure.contour and Figure.grdcontour. Probably it makes sense to bundle this change in one of these PRs and close the other one.

It seems that grdcontour and contour have identical -A and -C options. I think we can focus on improving grdcontour in this PR and apply the same changes to contour in PR #2706.

[yvonnefroehlich]

Looks good. Please also update the PR title to reflect the changes.

I updated the title (a bit long now) and the description in the initial comment of this PR.

[yvonnefroehlich]

Do we actually have to mention / warn that, due to removing sequence_plus for annotation arguments like [100, "f10p", "gred"] are no longer supported and "100+f10p+gred" should be used instead?

[seisman]

due to removing sequence_plus for annotation arguments like [100, "f10p", "gred"] are no longer supported and "100+f10p+gred" should be used instead?

The syntax is awkward. So maybe no one is actually using it. Or we may have to try hard to check if a list like [100, "f10p", "gred"] is passed.

[yvonnefroehlich]

/format