Skip to content

Figure: Refactor how plotting methods are attached to the Figure class#4612

Open
seisman wants to merge 2 commits intomainfrom
figure/class
Open

Figure: Refactor how plotting methods are attached to the Figure class#4612
seisman wants to merge 2 commits intomainfrom
figure/class

Conversation

@seisman
Copy link
Copy Markdown
Member

@seisman seisman commented Apr 29, 2026

Motivation

In PR #3849, we're seeing many static type check errors (https://github.com/GenericMappingTools/pygmt/actions/runs/25118862756/job/73613589767?pr=3849), mainly because in pygmtlogo, we're trying to re-import the Figure class, which causes some circular imports.

Codex recommends a different way to attach the plotting wrappers to the Figure class, which can solve the static type error found in PR #3849. Changes in this PR are made by Codex.

Summary

This PR refactors how plotting methods are attached to pygmt.Figure and clarifies the role of pygmt.src.

Why

Previously, Figure methods were attached through a class-scoped from pygmt.src import (...) block in pygmt/figure.py. That worked at runtime, but it had a few drawbacks:

  • It depended on package-level re-exports from pygmt/src/__init__.py just to populate Figure.
  • Static analysis could misinterpret some imported names as modules instead of callables. (see the mypy errors in found https://github.com/GenericMappingTools/pygmt/actions/runs/25118862756/job/73613589767?pr=3849)
  • The class-scoped import block was harder to read and maintain than explicit assignments.
  • It blurred two separate concepts:
    • standalone functions that are part of the functional API
    • plotting functions that are intended to be used as methods of Figure

What changed

pygmt/figure.py

  • Import each plotting callable directly from its concrete pygmt.src.* module.
  • Attach these callables explicitly on the Figure class body, e.g. plot = _plot.
  • Add a short comment above the attachment block to clarify the pattern.

pygmt/src/__init__.py

  • Keep only standalone functions that are intended to be called directly.
  • Stop re-exporting Figure-bound plotting methods from pygmt.src.
  • Add a short comment explaining the distinction between standalone functions and Figure methods.

Why this is better

This makes the API structure clearer:

  • pygmt.src becomes a cleaner namespace for standalone operations.
  • Figure is the clear home for plotting methods.

It also improves maintainability:

  • Each attached Figure method points to one explicit source module.
  • The attachment logic is easier to scan and modify.
  • Figure no longer depends on package-level re-export behavior for method attachment.
  • The structure is friendlier to static analysis and code review.

Behavioral intent

This is intended as an internal API-structure cleanup, not a plotting behavior change. The plotting implementations themselves are unchanged; only the way they are attached/exported has been reorganized.

@seisman seisman changed the title Refactor Figure method attachments Figure: Refactor how plotting methods are attached to the Figure class Apr 29, 2026
@seisman
Copy link
Copy Markdown
Member Author

seisman commented Apr 29, 2026

This PR is backward-compatible, but it's a big low-level refactor. Ping @GenericMappingTools/pygmt-maintainers for reviews.

Edit: Previously, it was possible to do from pygmt.src import plot, but it makes no sense to call plot as as standalone function. After this PR, from pygmt.src import plot no longer works.

@seisman seisman added the maintenance Boring but important stuff for the core devs label Apr 29, 2026
@seisman seisman added this to the 0.19.0 milestone Apr 29, 2026
@seisman seisman requested a review from a team April 29, 2026 17:53
@seisman seisman added the needs review This PR has higher priority and needs review. label Apr 29, 2026
@seisman
Copy link
Copy Markdown
Member Author

seisman commented Apr 30, 2026

This PR also renames the text_ method to text in commit d74da28. The name text_ was initially introduced to suppress a warning mentioned in #808 (comment).

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant