Figure.basemap: Migrate the box parameter to the new alias system and improve docstrings #4369
Conversation
… improve docstrings
seisman
added
maintenance
There was a problem hiding this comment.
Pull request overview
Migrates Figure.basemap’s box handling to the newer alias system and refreshes deprecation/docstring text for scale/rose-related parameters.
Changes:
- Add an explicit
boxparameter toFigure.basemapand route it throughAliasSystemvia-F. - Update
basemapdocstring wording/structure, including deprecation guidance toward higher-level APIs. - Tweak
coastdeprecation docstring text for themap_scaleparameter.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
pygmt/src/basemap.py |
Introduces box as an explicit parameter (mapped to -F) and updates documentation/deprecation messaging. |
pygmt/src/coast.py |
Minor docstring wording changes for deprecated map_scale. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pygmt/src/basemap.py
Outdated
| from pygmt.alias import Alias, AliasSystem | ||
| from pygmt.clib import Session | ||
| from pygmt.helpers import build_arg_list, fmt_docstring, use_alias | ||
| from pygmt.params.box import Box |
There was a problem hiding this comment.
Importing Box directly from pygmt.params.box is inconsistent with the rest of the codebase, which imports parameter classes from the pygmt.params package (e.g., pygmt/src/colorbar.py:from pygmt.params import Box). Prefer from pygmt.params import Box for consistency and to keep the public re-export as the canonical import path.
| Use the ``box`` parameter in :meth:`pygmt.Figure.scalebar`, | ||
| :meth:`pygmt.Figure.directional_rose`, or :meth:`pygmt.Figure.magnetic_rose` | ||
| instead. This parameter is maintained for backward compatibility and accepts | ||
| raw GMT CLI strings for the ``-F`` option. |
There was a problem hiding this comment.
The box parameter is typed as Box | str | bool, but its deprecation note says it only accepts raw GMT CLI strings for -F. Either document that Box instances are supported here (and how they map to -F modifiers), or remove Box from the accepted types to avoid a doc/API mismatch.
| raw GMT CLI strings for the ``-F`` option. | |
| either a :class:`~pygmt.params.box.Box` instance or raw GMT CLI strings for | |
| the ``-F`` option. |
| compass: str | None = None, | ||
| rose: str | None = None, | ||
| box: Box | str | bool = False, | ||
| verbose: Literal["quiet", "error", "warning", "timing", "info", "compat", "debug"] |
There was a problem hiding this comment.
A new public box parameter (including the new Box type) is introduced for Figure.basemap, but pygmt/tests/test_basemap.py doesn't cover it. Add at least one test that passes box=Box(...) (and ideally also checks the deprecated short-form F=... path) to prevent regressions in the alias migration.
Preview:
https://pygmt-dev--4369.org.readthedocs.build/en/4369/api/generated/pygmt.Figure.basemap.html#pygmt.Figure.basemap