Skip to content

fix: guard bw conversion in pl.spatial against missing image#4207

Open
steps-re wants to merge 1 commit into
scverse:mainfrom
steps-re:fix/spatial-bw-none-image
Open

fix: guard bw conversion in pl.spatial against missing image#4207
steps-re wants to merge 1 commit into
scverse:mainfrom
steps-re:fix/spatial-bw-none-image

Conversation

@steps-re

Copy link
Copy Markdown

Summary

sc.pl.spatial(..., bw=True) raises TypeError: 'NoneType' object is not subscriptable when called without an image (no img passed and no image available in .uns['spatial'], e.g. general spatial coordinates without a background image).

_check_img runs the grayscale conversion unconditionally whenever bw=True:

if bw:
    img = np.dot(img[..., :3], [0.2989, 0.5870, 0.1140])

If img is still None at that point, indexing it crashes. The existing test suite already documents this gap: test_manual_equivalency_no_img skips the bw case with the comment "Has no meaning when there is no image" instead of asserting it works.

Fix

Guard the conversion on img is not None, matching how the rest of _check_img and _check_na_color treat a possibly-missing image.

Test plan

  • Added test_spatial_bw_no_img, exercising sc.pl.spatial(..., bw=True) on data with no image anywhere, confirming it no longer raises.
  • Full tests/test_plotting_embedded/ suite passes locally (117 passed, 5 skipped, no change to skip count).
  • ruff check clean on changed files.
  • Added a towncrier fragment at docs/release-notes/4207.fix.md (will rename if the PR number differs).

sc.pl.spatial raises TypeError when bw=True is passed without an
image (no img argument and no image in .uns['spatial']). _check_img
ran the grayscale conversion unconditionally on bw=True, indexing
img even when it was still None.

Skip the conversion when there's no image to convert.

Signed-off-by: Mike German <mike@stepsventures.com>
@codecov

codecov Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.87%. Comparing base (7df9ad6) to head (4966e05).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4207      +/-   ##
==========================================
- Coverage   79.91%   79.87%   -0.04%     
==========================================
  Files         121      121              
  Lines       12949    12949              
==========================================
- Hits        10348    10343       -5     
- Misses       2601     2606       +5     
Flag Coverage Δ
hatch-test.low-vers ?
hatch-test.pre 79.87% <100.00%> (+<0.01%) ⬆️

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

Files with missing lines Coverage Δ
src/scanpy/plotting/_tools/scatterplots.py 83.56% <100.00%> (ø)

... and 2 files with indirect coverage changes

@steps-re

Copy link
Copy Markdown
Author

Heads up on the red CI: the only failing test is test_sim.py::test_sim_toggleswitch (ValueError: operands could not be broadcast together with shapes (100,2) (200,2)), which lives in sc.tl.sim / _sim.py. This PR only touches the bw image guard in scatterplots.py, so that failure is orthogonal to the change here. The plotting suite and the new test_spatial_bw_no_img regression test pass, and codecov is green. Happy to rebase if a fix for the sim test lands separately.

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