Skip to content

Refactor figure internals and add complexity reporting#608

Draft
cvanelteren wants to merge 2 commits intomainfrom
refactor/figure-complexity-ci
Draft

Refactor figure internals and add complexity reporting#608
cvanelteren wants to merge 2 commits intomainfrom
refactor/figure-complexity-ci

Conversation

@cvanelteren
Copy link
Collaborator

@cvanelteren cvanelteren commented Mar 10, 2026

This refactor is about making Figure easier to understand and easier to change without changing the public API. Over time, figure.py had become the place where almost everything happened at once: subplot creation, sharing rules, panel handling, guide placement, label alignment, formatting, layout queries, and various bits of policy. That made the file hard to navigate and made even small changes feel riskier than they should.

The main change in this PR is to turn Figure into more of a facade and state owner, while moving the detailed logic into focused internal helpers. The public interface stays the same, but the code behind it is now grouped by responsibility: guides, sharing, panels, labels, layout, formatting, factory logic, and option parsing each have a clearer home. That makes the structure easier to follow, makes the testing surface more modular, and gives us a much better base for future work.

I also added a lightweight complexity-report workflow for PRs. It doesn’t block anything, but it gives us visibility into whether a patch is making touched code better or worse. That felt worth doing alongside the refactor so we have a way to track this kind of cleanup going forward.

If you want, I can also give you a shorter version tailored for the actual GitHub PR description field.

Summary

  • split figure policy code into focused internal helpers so Figure acts as a thinner facade and state owner
  • move guide, sharing, panel, label, layout, factory, formatting, and option parsing logic out of figure.py
  • add a non-blocking complexity-report workflow that comments on touched Python blocks in patch scope

Complexity

  • reduce ultraplot/figure.py from 3655 lines to about 1700 lines
  • reduce radon cc average for ultraplot/figure.py from B (9.36) to A (1.79)
  • keep the extracted helper modules in the A/B range instead of moving large complexity wholesale

Verification

  • pytest ultraplot/tests/test_ci_complexity.py -q
  • pytest ultraplot/tests/test_figure.py -q
  • pytest ultraplot/tests/test_subplots.py -q
  • pytest ultraplot/tests/test_legend.py -q
  • pytest ultraplot/tests/test_colorbar.py -q
  • ruff check ultraplot/figure.py tools/ci/complexity_report.py ultraplot/internals/figure_factory.py ultraplot/internals/figure_formatting.py ultraplot/internals/figure_guides.py ultraplot/internals/figure_labels.py ultraplot/internals/figure_layout.py ultraplot/internals/figure_options.py ultraplot/internals/figure_panels.py ultraplot/internals/figure_sharing.py

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 refactors ultraplot.figure.Figure into a thinner facade by moving implementation details into focused internal helper modules, and adds a CI-oriented complexity reporting script + workflow to track cyclomatic complexity deltas for touched Python blocks.

Changes:

  • Split figure responsibilities into internal helpers (layout, sharing, panels, labels, guides, formatting, factory, options) while keeping the public API stable.
  • Updated figure-related tests to target the new helper-based internals and added targeted regression tests for renderer caching, guide placement, and border-axes classification.
  • Added a tools/ci/complexity_report.py utility, a GitHub Actions workflow to generate reports, and a unit test validating report behavior.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
ultraplot/figure.py Rewired Figure internals to delegate to helper objects (facade/state owner pattern).
ultraplot/internals/figure_factory.py Extracted projection parsing + subplot creation logic.
ultraplot/internals/figure_formatting.py Extracted figure-wide formatting orchestration.
ultraplot/internals/figure_guides.py Extracted legend/colorbar placement + span inference helpers.
ultraplot/internals/figure_labels.py Extracted super/spanning label alignment + share-label-group logic.
ultraplot/internals/figure_layout.py Extracted renderer + border/align layout queries and axes iteration.
ultraplot/internals/figure_options.py Centralized Figure constructor option parsing/normalization.
ultraplot/internals/figure_panels.py Extracted panel creation and label/tick policy.
ultraplot/internals/figure_sharing.py Extracted axis sharing compatibility + ticklabel sharing logic.
ultraplot/tests/test_figure.py Added/updated tests for new helper behaviors and moved internal call sites.
ultraplot/tests/test_subplots.py Updated label-group assertions to use the new label helper.
ultraplot/tests/test_ci_complexity.py Added test validating complexity report deltas on a temp git repo.
tools/ci/complexity_report.py New script to compute block-level CC deltas from a git diff using radon.
.github/workflows/complexity.yml New workflow to generate and publish the complexity report summary/artifacts.
pyproject.toml Updated Ruff configuration section layout.

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

Comment on lines +17 to +22
def test_complexity_report_tracks_touched_block_deltas(tmp_path):
repo = tmp_path / "repo"
repo.mkdir()
_git(repo, "init")
_git(repo, "config", "user.email", "ci@example.com")
_git(repo, "config", "user.name", "CI User")
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

This test executes tools/ci/complexity_report.py, which hard-requires the third-party radon package. radon is not listed in pyproject.toml dependencies/optional-dependencies or environment.yml, so this test will fail in the default CI test environment unless radon is already installed. Consider either adding radon to the project’s test/dev dependencies, or skipping this test when radon is unavailable (e.g., via an import-or-skip guard).

Copilot uses AI. Check for mistakes.
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.

2 participants