refactor: decompose show() into named stages (#697)#714
Draft
timtreis wants to merge 4 commits into
Draft
Conversation
…tion Decompose the show() god-function (#697). First, behavior-preserving steps: - pass _validate_show_parameters args by keyword so a signature reorder can no longer silently misvalidate one parameter as another - extract _collect_render_commands() and _normalize_title() module helpers No behavior change.
…bar stages Continue decomposing show() (#697), behavior-preserving: - _resolve_coordinate_systems(): CS auto-detection, validation and filtering - _plan_panels(): panel layout (one-per-CS vs one-per-color-key) + ax-count check - _build_legend_params(): LegendParams construction with legend_params overrides - promote the _draw_colorbar closure to a module function taking colorbar_params explicitly (was a ~90-line closure capturing it implicitly) - _layout_pending_colorbars(): the deferred second-pass colorbar layout No behavior change.
…_finalize_panel Extract the inner render-command dispatch loop (#697), behavior-preserving: - _render_panel(): dispatches each queued render command into one panel's axes, returning the wanted elements and per-type wants_* flags - _finalize_panel(): per-panel title / equal-aspect / frame visibility show() is now a compact orchestrator calling named stages. Full test suite passes unchanged (719 passed, 1 skipped), image baselines included. No behavior change.
Post-review cleanups (all behavior-preserving, full suite green 719 passed): - drop dead _draw_colorbar param base_offsets_axes (never read) - collapse the two parallel 4-branch location chains in _draw_colorbar into data-driven lookups (vertical flag + opposite map + getattr on the axis) - extract _should_rasterize() to dedup the images/labels rasterize heuristic - extract _maybe_set_label_colors() for the categorical-color prestep - replace the 5-branch render dispatch in _render_panel with a renderer table keyed by command; graph stays a small special case - pass cs_row instead of four has_* booleans; return a wants dict instead of a four-boolean tuple (removes the parallel-variable sprawl)
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #714 +/- ##
==========================================
+ Coverage 76.28% 76.39% +0.10%
==========================================
Files 14 14
Lines 4327 4325 -2
Branches 1006 994 -12
==========================================
+ Hits 3301 3304 +3
Misses 667 667
+ Partials 359 354 -5
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #697.
Decomposes the 644-line
show()god-function into named, single-purpose helpers. Behavior-preserving — no public API change.Extracted helpers
_collect_render_commands·_normalize_title·_resolve_coordinate_systems·_plan_panels·_build_legend_params·_draw_colorbar(promoted from closure) ·_layout_pending_colorbars·_render_panel·_finalize_panel·_should_rasterize·_maybe_set_label_colorsAlso:
_validate_show_parametersnow called by keyword; per-render dispatch is a table;has_*/wants_*boolean sprawl replaced bycs_row+ awantsdict.Metrics (
lizard,basic.py)show()cyclomatic complexityshow()NLOCVerification
show()benchmark (5-layer plot, n=25): median 720ms → 713ms — no regression (<1%, within noise).Out of scope
Bugs #694 (leaked
cs) and #695 (_finalize_panelruns per-command) are preserved as-is, now isolated for a clean follow-up.Generated by Claude Code