Skip to content

Conversation

@seisman
Copy link
Member

@seisman seisman commented May 12, 2025

The Figure._preprocess method was originally introduced in commit 4b414ce and later extended to wrap Figure._activate_figure in commit 5e69285.

The method was intended to take kwargs as input, modify them, and return the result. However, in practice, the input kwargs are never changed. As a result, Figure._preprocess serves only as a thin wrapper around Figure._activate_figure without adding any additional functionality.

This PR replaces all calls to Figure._preprocess with direct calls to Figure._activate_figure and deprecates the _preprocess method, which is scheduled for removal in four releases.

@seisman seisman added this to the 0.16.0 milestone May 12, 2025
@seisman seisman added deprecation Deprecating a feature needs review This PR has higher priority and needs review. labels May 12, 2025
@seisman seisman force-pushed the deprecate/preprocess branch from e11412a to 47a7e6a Compare May 12, 2025 01:50
@seisman seisman requested a review from Copilot May 12, 2025 05:34
Copy link
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

This PR deprecates the private _preprocess method by replacing its calls with direct invocations of _activate_figure, in preparation for removing _preprocess in v0.20.0. Key changes include updating all modules to remove the assignment from _preprocess, updating documentation in figure.py to warn users, and ensuring consistency across all call sites.

  • Removed calls that captured kwargs from _preprocess in favor of a direct call to _activate_figure.
  • Updated deprecation warnings and documentation in figure.py.
  • Applied changes consistently across multiple modules (e.g., rose.py, psconvert.py, contour.py, etc.).

Reviewed Changes

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

Show a summary per file
File Description
pygmt/src/shift_origin.py Replaced self._preprocess() with self._activate_figure().
pygmt/src/rose.py Removed kwargs assignment from self._preprocess(**kwargs) and replaced it with self._activate_figure().
pygmt/src/psconvert.py Updated call from self._preprocess(**kwargs) to self._activate_figure().
pygmt/src/plot3d.py Removed kwargs processing via _preprocess and called _activate_figure() instead.
pygmt/src/plot.py Replaced disappearing kwargs assignment with a bare _activate_figure() call.
pygmt/src/meca.py Updated the method call to use _activate_figure() for figure activation.
pygmt/src/logo.py Replaced _preprocess with _activate_figure() in logo initialization.
pygmt/src/legend.py Updated to call _activate_figure() rather than reassigning kwargs.
pygmt/src/inset.py Removed kwargs modification from _preprocess, calling _activate_figure() instead.
pygmt/src/image.py Updated _preprocess call to _activate_figure() for image plotting.
pygmt/src/hlines.py Changed _preprocess to _activate_figure() when plotting horizontal lines.
pygmt/src/histogram.py Replaced assignment from _preprocess with a direct _activate_figure() call.
pygmt/src/grdview.py Updated method call following the deprecation of _preprocess.
pygmt/src/grdimage.py Removed kwargs reassignment from _preprocess in favor of _activate_figure().
pygmt/src/grdcontour.py Replaced _preprocess with _activate_figure() for grdcontour plotting.
pygmt/src/contour.py Updated call site to use _activate_figure() instead of capturing kwargs from _preprocess().
pygmt/src/colorbar.py Removed return value dependency on _preprocess and directly activated the figure.
pygmt/src/coast.py Replaced _preprocess call with _activate_figure() to streamline figure activation.
pygmt/src/basemap.py Updated the call to use _activate_figure() instead of processing through _preprocess.
pygmt/figure.py Added deprecation warning in _preprocess and guided users to use _activate_figure().
Comments suppressed due to low confidence (3)

pygmt/src/rose.py:201

  • The call to _preprocess that reassigns kwargs has been replaced with a call to _activate_figure(), which no longer returns modified kwargs. Please confirm that downstream code does not rely on any modifications to kwargs returned from _preprocess.
kwargs = self._preprocess(**kwargs)

pygmt/src/psconvert.py:110

  • Replacing the _preprocess call with _activate_figure() removes the reassignment of kwargs. Verify that no functionality is impacted by not capturing updated kwargs.
kwargs = self._preprocess(**kwargs)

pygmt/figure.py:130

  • [nitpick] The deprecation warning in _preprocess is clear; consider reviewing the version numbers in both the comment and warning message for consistency to ensure future maintainability.
warnings.warn( ... )

pygmt/figure.py Outdated
with Session() as lib:
lib.call_module(module="figure", args=[self._name, fmt])

# TODO(PyGMT>=v0.20.0): Remove the _preprocess method.
Copy link
Member

Choose a reason for hiding this comment

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

Technically this _preprocess method is private so we could just remove it 🔥 But if you think people are using it, we could do with 2 release cycles instead of 4.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, people are using it. 2 release cycles sounds good to me.

@weiji14 weiji14 changed the title Figure: Deprecated the private _preprocess method (will be removed in v0.20.0) Figure: Deprecate the private _preprocess method (will be removed in v0.20.0) May 14, 2025
@seisman seisman changed the title Figure: Deprecate the private _preprocess method (will be removed in v0.20.0) Figure: Deprecate the private _preprocess method (will be removed in v0.18.0) May 14, 2025
@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. final review call This PR requires final review and approval from a second reviewer labels May 16, 2025
@seisman seisman merged commit 528cf2a into main May 16, 2025
23 of 26 checks passed
@seisman seisman deleted the deprecate/preprocess branch May 16, 2025 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deprecation Deprecating a feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants