Skip to content

BREAKING: Figure.coast: Add parameter river_lakes for setting fills of river-lakes separately - #4376

Open
seisman wants to merge 16 commits into
mainfrom
coast/lakes
Open

BREAKING: Figure.coast: Add parameter river_lakes for setting fills of river-lakes separately#4376
seisman wants to merge 16 commits into
mainfrom
coast/lakes

Conversation

@seisman

@seisman seisman commented Jan 29, 2026

Copy link
Copy Markdown
Member

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 same fill for 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 in the old syntax, using lakes="blue" also set the fill color of river-lakes to "blue". If we preserved that behavior, there would be no Pythonic way to fill only lakes without also filling river-lakes. Therefore, I decided to alias lakes="blue" to -Cblue+l, meaning “fill lakes only.” This introduces a breaking change.

Related to #4240

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment thread pygmt/src/coast.py
Comment thread pygmt/src/coast.py
Comment thread pygmt/src/coast.py
Comment thread pygmt/src/coast.py
Comment thread pygmt/src/coast.py Outdated
@seisman
seisman marked this pull request as draft January 29, 2026 12:08
@seisman seisman changed the title Figure.coast: Improve parameters lakes/river_lakes for setting fill of lakes/river-lakes BREAKING: Figure.coast: Add parameter river_lakes for setting fills of river-lakes separately May 17, 2026
@seisman seisman added enhancement Improving an existing feature needs review This PR has higher priority and needs review. labels May 17, 2026
@seisman seisman added this to the 0.19.0 milestone May 17, 2026
@seisman
seisman marked this pull request as ready for review May 17, 2026 11:08
@seisman seisman removed the needs review This PR has higher priority and needs review. label May 22, 2026
@seisman seisman removed this from the 0.19.0 milestone May 23, 2026
@seisman seisman added this to the 0.20.0 milestone Jul 23, 2026
@seisman seisman added the needs review This PR has higher priority and needs review. label Jul 23, 2026
@seisman seisman 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 Aug 9, 2026
@seisman
seisman requested a review from a team August 9, 2026 09:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (5)

pygmt/src/coast.py:306

  • The “at least one plotting parameter” guard now checks kwargs.get("C", lakes or river_lakes) is None, which can behave differently from the previous args_in_kwargs-based check (e.g., C=False will bypass the guard even though it’s treated as “not given” elsewhere). Consider explicitly checking lakes/river_lakes and using args_in_kwargs for short-form C for consistent semantics.
        and kwargs.get("W", shorelines) is False
        and kwargs.get("C", lakes or river_lakes) is None
        and not args_in_kwargs(args=["E", "Q"], kwargs=kwargs)

pygmt/src/coast.py:323

  • This PR introduces new parsing/aliasing behavior for GMT’s -C option (including legacy compatibility and mixed-usage errors), but there are no unit tests covering the new lakes/river_lakes parameters. Adding tests would help prevent regressions in argument translation and breaking-change semantics.
    aliasdict = AliasSystem(
        C=_alias_option_C(lakes=lakes, river_lakes=river_lakes),

pygmt/src/coast.py:63

  • Typo in the inline comment: “sytax” should be “syntax” (helps keep comments searchable/readable).
    # The new sytax.

pygmt/src/coast.py:85

  • lakes/river_lakes were inserted in the middle of the public coast signature, which creates an additional breaking change for any positional callers (arguments after shorelines shift). To minimize breakage, append the new parameters near the end of the signature (before **kwargs) so existing positional argument order is preserved.
    shorelines: bool | str | Sequence[int | str] = False,
    lakes: str | None = None,
    river_lakes: str | None = None,
    map_scale: str | None = None,
    box: Box | bool = False,

pygmt/src/coast.py:149

  • The new docstrings for lakes/river_lakes don’t mention the accepted legacy -C syntax ("fill+l", "fill+r", or a list of these) that _alias_option_C still supports. This can confuse users/type checkers during migration.
    lakes
        Select filling of lakes. If not specified, will use the fill for "wet" areas set
        by the ``water`` parameter.
    river_lakes
        Select filling of river-lakes. If not specified, will use the fill for "wet"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improving an existing feature final review call This PR requires final review and approval from a second reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants