Share the normalization bounds between rgb() and colorbar() - #18
Merged
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 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
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.
When
spd_minorspd_maxare not given, they are computed withnumpy.nanmin/numpy.nanmaxover the entire spectral cube.rgb_and_colorbar()was doing this six times:rgb()computed the bounds once, andcolorbar()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 bothrgb()andcolorbar().The bounds were already being resolved with
_bounds_normalize()in every path, so the values are unchanged.colorbar()previously resolved them once against the rawaxisand once against the negative-normalizedaxis_; these select the same axis in every case that works today, and unifying onaxis_additionally avoids anIndexErrorwhen a positiveaxisis used with anspdthat has fewer dimensions than the broadcast shape.Measured effect, (512, 512, 101) cube, mean of 5 runs:
rgb_and_colorbar()colorbar()alonergb()aloneThe output is unchanged: the checksums of both the RGB array and the colorbar match the previous implementation to all printed digits.
Test changes
test_rgb_and_colorbar_equivalence, which asserts withnumpy.array_equalthatrgb_and_colorbar()returns exactly what independentrgb()andcolorbar()calls return, across several shapes, a non-default axis, a unit-bearing input, andwavelength=None. This is the guard for the whole refactor.test_colorbar_duplicate_wavelength, added in Build thecolorbar()diagonal from indices and add anum_intensityparameter #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-dimensionalspd, for which the automatic bounds degenerate tovmin == vmaxand 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.🤖 Generated with Claude Code