Skip to content

perf: avoid dense BasePlot aggregation - #4399

Open
srikarjy wants to merge 6 commits into
scverse:mainfrom
srikarjy:issue-3718-baseplot-aggregate
Open

srikarjy wants to merge 6 commits into
scverse:mainfrom
srikarjy:issue-3718-baseplot-aggregate

Conversation

@srikarjy

@srikarjy srikarjy commented Oct 3, 2026

Copy link
Copy Markdown

Summary

  • preserve sparse and backed plotting inputs instead of eagerly building a dense cell-level dataframe in BasePlot

  • aggregate dotplot sizes/colors and matrixplot values with Scanpy aggregation kernels

  • keep obs_tidy lazy for plot types that still require cell-level values

  • preserve mean_only_expressed semantics for nonzero expression cutoffs

  • Closes Refactor BasePlot not to create a dataframe representation of the data #3718

  • Tests included

  • Release notes not necessary because: a performance release note is included

Testing

  • uv run ruff check on changed Python files
  • uv run ruff format --check on changed Python files
  • .venv/bin/python -m pytest tests/plotting/legacy/test_plotting.py -k "dotplot or matrixplot or stacked_violin" --disable-pytest-warnings -q (50 passed, 5 skipped, 2 xfailed)
  • complete legacy plotting module: 129 passed, 18 skipped, 6 xfailed; three unrelated violin image comparisons exceeded the local pixel tolerance

@codecov

codecov Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.78261% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.88%. Comparing base (7ad567d) to head (12dd884).

Files with missing lines Patch % Lines
src/scanpy/plotting/legacy/_baseplot_class.py 94.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4399      +/-   ##
==========================================
+ Coverage   83.78%   83.88%   +0.10%     
==========================================
  Files         134      134              
  Lines       12691    12796     +105     
==========================================
+ Hits        10633    10734     +101     
- Misses       2058     2062       +4     
Flag Coverage Δ
hatch-test.low-vers 80.62% <94.78%> (+0.12%) ⬆️
hatch-test.pre 83.69% <94.78%> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/scanpy/plotting/legacy/_dotplot.py 92.39% <100.00%> (+0.50%) ⬆️
src/scanpy/plotting/legacy/_matrixplot.py 96.59% <100.00%> (ø)
src/scanpy/plotting/legacy/_baseplot_class.py 83.82% <94.00%> (+3.46%) ⬆️

... and 1 file with indirect coverage changes

@srikarjy
srikarjy force-pushed the issue-3718-baseplot-aggregate branch from 0f0fbee to 12dd884 Compare October 5, 2026 17:56

This branch has not been deployed

No deployments
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.

Refactor BasePlot not to create a dataframe representation of the data

1 participant