Skip to content

Chore: refactor figure with new subplot manager#698

Merged
cvanelteren merged 11 commits into
v3.0from
chore/refactor-subplotmanager-figure
Jul 14, 2026
Merged

Chore: refactor figure with new subplot manager#698
cvanelteren merged 11 commits into
v3.0from
chore/refactor-subplotmanager-figure

Conversation

@cvanelteren

Copy link
Copy Markdown
Collaborator

Partially addresses #677

This PR marks one of the first changes behind the scenes for the Figure class. The idea is to use Figuremore as an interface with clear separation of tasks and responsibility. This will change the monolith it is today to better mangeable code that can be edited, expanded, and easier contributed to.

@codecov

codecov Bot commented Apr 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 74.59016% with 62 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
ultraplot/_subplots.py 72.97% 36 Missing and 24 partials ⚠️
ultraplot/figure.py 88.88% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@cvanelteren
cvanelteren marked this pull request as draft April 20, 2026 03:47
@cvanelteren
cvanelteren changed the base branch from main to v3.0 April 22, 2026 04:19
@cvanelteren
cvanelteren marked this pull request as ready for review April 22, 2026 04:39
Figure._parse_proj had been reduced to a (*args, **kwargs) passthrough to
SubplotManager. That signature is introspected, not just called: ui.subplot
runs _pop_params over it to decide which keywords belong to the subplot
rather than the figure. With the named parameters gone, proj, projection,
proj_kw, projection_kw, backend and basemap were routed to the figure
instead, so uplt.subplot(proj="polar") raised

    AttributeError: Figure.set() got an unexpected keyword argument 'proj'

Spell the parameters out again and forward them by name. uplt.subplots was
unaffected only because _add_subplots kept its explicit signature.

Also in _subplots.py: drop the unused inspect and typing imports, resolve
the Figure annotation under TYPE_CHECKING, use super() instead of naming
matplotlib's Figure so a mixin in a subclass MRO is not skipped, and remove
the dead isinstance(figure, Axes) branches (a Figure is never an Axes).

Cover the ui.subplot/ui.subplots projection paths, which no test exercised,
and tighten the manager tests that only asserted "is not None" or len(axs)
so they can actually fail. Note that the external-container test must use a
projection cartopy does not also know, or it resolves to a GeoAxes instead.
@cvanelteren
cvanelteren merged commit b22a041 into v3.0 Jul 14, 2026
7 checks passed
@cvanelteren
cvanelteren deleted the chore/refactor-subplotmanager-figure branch July 14, 2026 02:59
cvanelteren added a commit that referenced this pull request Jul 14, 2026
Move subplot creation, gridspec ownership, and projection parsing out of `Figure` and into a dedicated `SubplotManager` (`ultraplot/_subplots.py`), a first step toward treating `Figure` as an interface that delegates to focused collaborators rather than a monolith (partially addresses #677). The public API is unchanged: the former `_subplot_dict`, `_subplot_counter` and `_gridspec` attributes now live on the manager and are reached through the new `Figure._get_subplot()` / `Figure._iter_subplots()` accessors and the existing `Figure.gridspec` property, with call sites in `gridspec.py` and `axes/base.py` going through those instead of touching figure internals. Also fixes projection keywords being dropped by `uplt.subplot()`: `ui.subplot` introspects the signature of `Figure._parse_proj` to decide which keywords belong to the subplot rather than the figure, so that signature must stay spelled out rather than collapsing into `**kwargs`. Adds `ultraplot/tests/test_subplot_manager.py` covering subplot creation, the gridspec setter, projection resolution (native, external-container, and geographic), per-axes projection arguments, and the `ui.subplot`/`subplots` keyword routing. Follow-ups filed: #755 (signature coupling in `ui.py`), #756 (`Figure.clear` leaves stale subplot state), #757 (`SubplotManager`/`Figure` coupling), #758 (dead imports).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant