Figure.coast: Improve parameters lakes/river_lakes for setting fill of lakes/river-lakes

[seisman]

This PR improves the alias of coast's -C option. The GMT CLI syntax is -Cfill[+l|+r].

Set the shade, color, or pattern for lakes and river-lakes [Default is the fill chosen for “wet” areas -S]. Optionally, specify separate fills by appending +l for lakes or +r for river-lakes, repeating the -C option as needed.

Previously, -C was aliased to lakes. This PR split the -C option into two parameters, lakes and river_lakes. Here is a comparison of old and new usages:

Description	Old	New
Set fill for both lakes and river-lakes	lakes="blue"	lakes="blue", river_lakes="blue"
Set fill for river-lakes only	lakes="blue+r"	river_lakes="blue"
Set fill for lakes only	lakes="blue+l"	lakes="blue"
Set separate fills for river-lakes and lakes	lakes=["blue+r", "green+l"]	lakes="green", river_lakes="blue"

Please note that lakes="blue" has different behaviors in the old and new syntax. So, it's a breaking change.

[Copilot]

Pull request overview

This PR refactors Figure.coast’s handling of GMT’s -C option to provide a clearer long-form API for setting lake vs river-lake fills, while preserving backward compatibility with the existing lakes=[...] list syntax.

Changes:

• Added a dedicated _alias_option_C helper to build -C arguments from lakes/river_lakes, including a compatibility path for the legacy list form.
• Introduced new river_lakes parameter and updated coast validation logic and alias mapping accordingly.
• Updated docstring alias/parameter documentation around lake and river-lake fills.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

_alias_option_C returns a list of Alias objects where both entries have no prefix (only suffix). When users pass short-form C=... via kwargs, AliasSystem.merge will build a warning/error message for a “sequence of aliases” that includes an empty “with optional parameters …” clause (see pygmt/alias.py around the _modifiers join). To avoid degraded/confusing messages for C, consider returning a single Alias (with its value computed as a string or list like ["blue+l", "cyan+r"]) instead of a list of Alias objects, or otherwise ensuring the merge message has meaningful content for this option.

Suggested change

	return Alias(lakes, name="lakes") # Return as is.
	# If only 'lakes' is specified, no suffix is needed.
	return [
	Alias(lakes, name="lakes", suffix="+l" if river_lakes is not None else ""),
	Alias(river_lakes, name="river_lakes", suffix="+r"),
	]
	# Pass through the legacy list value unchanged as the argument to -C.
	return Alias(lakes, name="C")
	# New syntax: lakes and/or river_lakes are given as strings.
	# We construct the final value for -C directly so that AliasSystem does not
	# need to merge a sequence of aliases without prefixes.
	values = []
	if lakes is not None:
	lake_val = str(lakes)
	# If both lakes and river_lakes are specified and no modifier is present,
	# append +l for lakes (for backward compatibility with the previous logic).
	if (
	river_lakes is not None
	and "+l" not in lake_val
	and "+r" not in lake_val
	):
	lake_val = f"{lake_val}+l"
	values.append(lake_val)
	if river_lakes is not None:
	river_val = str(river_lakes)
	# Append +r only if the user did not already include modifiers.
	if "+r" not in river_val and "+l" not in river_val:
	river_val = f"{river_val}+r"
	values.append(river_val)
	if not values:
	# No value for -C.
	return Alias(None, name="C")
	combined = values[0] if len(values) == 1 else values
	return Alias(combined, name="C")

[Copilot]

The “required args” validation uses kwargs.get("C", lakes or river_lakes) is None. Using or means a falsy but explicitly provided lakes value (e.g., "") would be treated as not provided. Using an explicit None check (e.g., prefer lakes if lakes is not None else river_lakes) avoids truthiness pitfalls and matches how the other checks treat values.

Suggested change

	and kwargs.get("C", lakes or river_lakes) is None
	and kwargs.get("C", lakes if lakes is not None else river_lakes) is None

[Copilot]

New behavior for lakes/river_lakes (including suffix selection and the backward-compatible lakes=["...+l", "...+r"] form and the mixed-usage error) isn’t covered by tests. pygmt/tests/test_coast.py currently tests required-args and dcw/resolution conflicts but has no assertions around -C argument building. Please add tests that exercise: (1) lakes="blue" emits -Cblue, (2) river_lakes="cyan" emits -Ccyan+r, (3) both emit two -C... entries with +l/+r, and (4) mixed list + river_lakes raises GMTInvalidInput.

[Copilot]

The docstring parameter entries for lakes/river_lakes don’t follow the numpydoc pattern used elsewhere in the file (missing : type on the name line and the shared description is indented under river_lakes only). This can break/garble rendered docs. Suggest documenting as either lakes, river_lakes : str with a single indented description, or provide separate lakes : str / river_lakes : str blocks (and mention the backward-compatible list-of-strings form if it remains supported).

Suggested change

	lakes
	river_lakes
	lakes, river_lakes : str

[Copilot]

The docstring says “If lakes is specified but river_lakes isn’t, river_lakes will use the same fill as lakes.” This is true for the new API when lakes is a plain fill (emits -C<fill>), but it’s not true for the documented backward-compatible modifier forms like lakes="blue+l" (which intentionally affects lakes only). Consider clarifying that the inheritance behavior applies when lakes is given without an explicit +l/+r modifier.

Suggested change

	"wet" areas set by the ``water`` parameter. If ``lakes`` is specified but
	``river_lakes`` isn't, ``river_lakes`` will use the same fill as ``lakes``.
	"wet" areas set by the ``water`` parameter. If ``lakes`` is specified as a plain
	fill (without an explicit ``+l``/``+r`` modifier) but ``river_lakes`` isn't,
	``river_lakes`` will use the same fill as ``lakes``.