Hi, I wanted to check whether maintainers would be open to a small behavior-preserving refactor in QuickPlot.
While looking at src/pybamm/plotting/quick_plot.py, I noticed that the 2D spatial plotting decisions are currently spread across set_output_variables, reset_axis, plot, and slider_update.
Those paths all participate in the same local planning decision: Which spatial variable goes on each axis; whether 2D data is transposed, etc.
I have a small local refactor that centralizes that planning into a private _QuickPlot2DSpatialPlan inside quick_plot.py. With the intent being to reduce duplicated 2D planning logic. It does not change plotting behavior or public API. QuickPlot would still own the actual Matplotlib rendering, stored plot handles/data, axis limits, variable limits, and slider update flow.
The PR would preserve all previous behaviours.
I also added focused characterization assertions for the behavior that was not directly covered before: 2D labels, axis extent order, draw handle type, and the readable compatibility flags.
Focused tests pass locally with: python -m pytest tests/unit/test_plotting/test_quick_plot.py -q
14 passed
The scope is limited to:
src/pybamm/plotting/quick_plot.py
tests/unit/test_plotting/test_quick_plot.py
Would maintainers be open to a small PR for this refactor?
Hi, I wanted to check whether maintainers would be open to a small behavior-preserving refactor in QuickPlot.
While looking at src/pybamm/plotting/quick_plot.py, I noticed that the 2D spatial plotting decisions are currently spread across set_output_variables, reset_axis, plot, and slider_update.
Those paths all participate in the same local planning decision: Which spatial variable goes on each axis; whether 2D data is transposed, etc.
I have a small local refactor that centralizes that planning into a private _QuickPlot2DSpatialPlan inside quick_plot.py. With the intent being to reduce duplicated 2D planning logic. It does not change plotting behavior or public API. QuickPlot would still own the actual Matplotlib rendering, stored plot handles/data, axis limits, variable limits, and slider update flow.
The PR would preserve all previous behaviours.
I also added focused characterization assertions for the behavior that was not directly covered before: 2D labels, axis extent order, draw handle type, and the readable compatibility flags.
Focused tests pass locally with: python -m pytest tests/unit/test_plotting/test_quick_plot.py -q
14 passed
The scope is limited to:
src/pybamm/plotting/quick_plot.py
tests/unit/test_plotting/test_quick_plot.py
Would maintainers be open to a small PR for this refactor?