Skip to content

Figure.text: Migrate the fill/pen parameters to the new alias system#4489

Merged
seisman merged 4 commits intomainfrom
text/fill-pen
Apr 3, 2026
Merged

Figure.text: Migrate the fill/pen parameters to the new alias system#4489
seisman merged 4 commits intomainfrom
text/fill-pen

Conversation

@seisman
Copy link
Copy Markdown
Member

@seisman seisman commented Apr 2, 2026

Just regular migration.

@seisman seisman added this to the 0.19.0 milestone Apr 2, 2026
@seisman seisman added maintenance Boring but important stuff for the core devs skip-changelog Skip adding Pull Request to changelog final review call This PR requires final review and approval from a second reviewer labels Apr 2, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Migrates Figure.text/text_ handling of the GMT -G (fill) and -W (pen) options from the legacy @use_alias mechanism to the newer AliasSystem-based alias handling.

Changes:

  • Removed G="fill" and W="pen" from the @use_alias decorator and introduced explicit fill/pen keyword parameters.
  • Added fill/pen into the AliasSystem construction so they are emitted as -G/-W.
  • Tightened type annotations for angle and font, and added an inline type: ignore for a mixed-type list used in validation.

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

seisman and others added 2 commits April 2, 2026 22:03
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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.


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

@seisman seisman merged commit c0dd933 into main Apr 3, 2026
23 of 25 checks passed
@seisman seisman deleted the text/fill-pen branch April 3, 2026 02:28
@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Apr 3, 2026
angle=None,
font=None,
angle: float | Sequence[float] | bool = False,
font: str | StringArrayTypes | bool = False,
Copy link
Copy Markdown
Member

@yvonnefroehlich yvonnefroehlich Apr 11, 2026

Choose a reason for hiding this comment

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

I am wondering about the context of the type bool for angle and font. Why was there a change from None to False? And I am wondering if this causes the newly occuring warning by the Figure.text method, e.g., https://www.pygmt.org/dev/tutorials/basics/text.html:

text [WARNING]: Length <unit> e not supported - revert to default unit [point]
text [WARNING]: Fals not a valid number and may not be decoded properly.
text [WARNING]: Representation of font size not recognized. Using default.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I am wondering about the context of the type bool for angle and font. Why was there a change from None to False?

In Figure.text, angle=True, font=True means the input file contains columns for angle and font, so angle/font can accept bools.

The issue is that when changing the default from None to False, I didn't update the checking of these parameters. #4577 should fix it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In Figure.text, angle=True, font=True means the input file contains columns for angle and font, so angle/font can accept bools.

Ah, OK. Thanks!

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 skip-changelog Skip adding Pull Request to changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants