Skip to content

Share the normalization bounds between rgb() and colorbar() - #18

Merged
roytsmart merged 1 commit into
mainfrom
share-normalization-bounds
Aug 19, 2026
Merged

Share the normalization bounds between rgb() and colorbar()#18
roytsmart merged 1 commit into
mainfrom
share-normalization-bounds

Conversation

@roytsmart

Copy link
Copy Markdown
Contributor

When spd_min or spd_max are not given, they are computed with numpy.nanmin/numpy.nanmax over the entire spectral cube. rgb_and_colorbar() was doing this six times: rgb() computed the bounds once, and colorbar() computed them twice, once inside _transform_spd_wavelength() and again for its own colorbar construction.

This PR computes each unspecified bound once and passes it down:

  • colorbar() resolves the bounds up front and reuses them for both purposes, halving its own cost even when called on its own.
  • rgb_and_colorbar() resolves the spectral and wavelength bounds once and passes them to both rgb() and colorbar().

The bounds were already being resolved with _bounds_normalize() in every path, so the values are unchanged. colorbar() previously resolved them once against the raw axis and once against the negative-normalized axis_; these select the same axis in every case that works today, and unifying on axis_ additionally avoids an IndexError when a positive axis is used with an spd that has fewer dimensions than the broadcast shape.

Measured effect, (512, 512, 101) cube, mean of 5 runs:

benchmark before after
rgb_and_colorbar() 913.7 ms 602.5 ms
colorbar() alone 364.9 ms 202.0 ms
rgb() alone 546.5 ms 556.4 ms

The output is unchanged: the checksums of both the RGB array and the colorbar match the previous implementation to all printed digits.

Test changes

  • Adds test_rgb_and_colorbar_equivalence, which asserts with numpy.array_equal that rgb_and_colorbar() returns exactly what independent rgb() and colorbar() calls return, across several shapes, a non-default axis, a unit-bearing input, and wavelength=None. This is the guard for the whole refactor.
  • Rewrites test_colorbar_duplicate_wavelength, added in Build the colorbar() diagonal from indices and add a num_intensity parameter #16, which was fragile. It drew from the module-level generator inside the test body, so its input depended on how many random values had been consumed before it ran, and adding any test that draws at import time changed its data. It also used a one-dimensional spd, for which the automatic bounds degenerate to vmin == vmax and the normalization divides by zero, so it was comparing arrays containing NaN and infinity and passed only for particular draws. It now uses a fixed array with explicit bounds, asserts the result is finite, and compares to a tolerance of 1e-4 rather than 1e-3.
  • Gives the two other tests that drew inside the test body their own seeded generators, so no test depends on execution order.

🤖 Generated with Claude Code

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (525a685) to head (709e091).

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #18   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines          403       420   +17     
=========================================
+ Hits           403       420   +17     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

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

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@roytsmart
roytsmart merged commit 0e6a894 into main Aug 19, 2026
16 checks passed
@roytsmart
roytsmart deleted the share-normalization-bounds branch August 19, 2026 19:44
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