Fix several issues with the bqplot viewer#207
Merged
Conversation
The bqplot widget renders a freshly loaded image with min/max scaling instead of the percentile cuts the API layer stores on load, so images with a few bright pixels display as all black. Co-written with Claude Fable 5 Claude-Session: https://claude.ai/code/session_01D5TYrgNKZUP6nDV3ds99T7
The API layer stores percentile cuts and a linear stretch while loading, but its set_cuts/set_stretch calls fire before self._data is assigned, so they never reach the screen. load_image then sent the data with no cuts, falling back to MinMaxInterval, which renders typical astronomical images (narrow background plus a few bright pixels) as all black. Add a _refresh_display method that recomputes the displayed array from the cuts and stretch stored for the image, and call it from load_image once the data is in place, so a display refresh works the same no matter what triggered it. Drop the self._cuts attribute that nothing ever read. Co-written with Claude Fable 5 Claude-Session: https://claude.ai/code/session_01D5TYrgNKZUP6nDV3ds99T7
The widget-level fallback cuts are MinMaxInterval, which renders a typical astronomical image (narrow background plus a few bright pixels) as nearly all black. The fallback should instead cut out the sky background at the bottom and clip only the brightest pixels at the top; the 30-96 percentile interval looks good in practice. Co-written with Claude Fable 5 Claude-Session: https://claude.ai/code/session_01D5TYrgNKZUP6nDV3ds99T7
Replace the MinMaxInterval fallback with cuts that remove the sky background below the 30th percentile and clip the brightest pixels above the 96th. Co-written with Claude Fable 5 Claude-Session: https://claude.ai/code/session_01D5TYrgNKZUP6nDV3ds99T7
Nothing currently sets a colormap: the API layer leaves the per-image colormap as None on load, and the bqplot ColorScale falls back to the d3 'Greys' scheme, which maps low values to white -- the inverse of the usual astronomical convention. Test that a fresh widget and a freshly loaded image both display with Greys_r and that get_colormap reports it. Co-written with Claude Fable 5 Claude-Session: https://claude.ai/code/session_01D5TYrgNKZUP6nDV3ds99T7
Apply Greys_r to the color scale when the widget is constructed so even an empty viewer starts with the conventional low-is-black mapping, and record it via set_colormap when a freshly loaded image has no colormap stored. Co-written with Claude Fable 5 Claude-Session: https://claude.ai/code/session_01D5TYrgNKZUP6nDV3ds99T7
set_stretch passes cuts=None to _send_data, so changing the stretch silently swaps the stored percentile cuts for the widget's fallback cuts -- the same class of bug load_image had before this branch fixed it there. set_cuts drops the stored stretch the same way. Co-written with Claude Fable 5 Claude-Session: https://claude.ai/code/session_01D5TYrgNKZUP6nDV3ds99T7
set_stretch sent the new stretch with cuts=None and set_cuts sent the new cuts with stretch=None, so changing one setting silently swapped the stored value of the other for the widget's defaults. Have both setters call _refresh_display, which reads the stored cuts and stretch itself, so a display refresh works the same no matter which setting changed. Co-written with Claude Fable 5 Claude-Session: https://claude.ai/code/session_01D5TYrgNKZUP6nDV3ds99T7
Loading an image assigns many traitlets in sequence -- the viewport initialization moves the scales while the old image is still shown, then the image data, x, and y extents update one message at a time. Each sync triggers a browser redraw, so switching images flickers through several visible intermediate states. Test that loading an image sends at most one state message per widget (the image mark and each scale) without changing the end state. The send_state spies come from pytest-mock's mocker fixture; declare pytest-mock in the test extras. Co-written with Claude Fable 5 Claude-Session: https://claude.ai/code/session_01D5TYrgNKZUP6nDV3ds99T7
Add a context manager to _AstroImage that holds the ipywidgets sync for the image mark and both scales, and use it in set_data and around the whole of load_image. Each widget now sends a single state message per load, so the browser no longer redraws the old image at the new viewport, then the new image at the old extent, on its way to the final state. The scale changes made by the API layer's viewport initialization (via the set_viewport override) land in the same batch as the new image data. Python-side observers are unaffected; hold_sync only defers messages to the front end, and it is re-entrant so the nested use in set_data is safe. Co-written with Claude Fable 5 Claude-Session: https://claude.ai/code/session_01D5TYrgNKZUP6nDV3ds99T7
Loading a new image should display it with whatever cuts, stretch and colormap are currently in effect (the widget defaults if nothing has been displayed yet) and store them for the new image, instead of letting the API layer reset cuts and stretch and resetting the colormap to the default. Only the viewport resets on load. Co-written with Claude Fable 5 Claude-Session: https://claude.ai/code/session_01D5TYrgNKZUP6nDV3ds99T7
Capture the settings in effect before the load, store them for the new image (overriding the defaults the API layer stores while loading) and display with them. On the first load, when nothing has been displayed yet, the widget defaults are used instead. Only the viewport resets when an image is loaded. The settings are stored through the widget's own set_cuts / set_stretch / set_colormap. A new _defer_refresh context manager suspends the display refresh the setters trigger, so the displayed array is still computed only once per load, in the single _refresh_display call at the end. Co-written with Claude Fable 5 Claude-Session: https://claude.ai/code/session_01D5TYrgNKZUP6nDV3ds99T7
load_image carries the settings forward from the most recently loaded image even when the caller passes an image_label that already has its own stored cuts, stretch and colormap. Those are the current settings for that image and are the ones that should be kept on reload. Co-written with Claude Fable 5 Claude-Session: https://claude.ai/code/session_01D5TYrgNKZUP6nDV3ds99T7
If the caller passes an image_label that already has stored settings, those are the current settings for that image and are the ones kept, instead of always carrying forward the most recently loaded image's settings. Co-written with Claude Fable 5 Claude-Session: https://claude.ai/code/session_01D5TYrgNKZUP6nDV3ds99T7
The carry-forward of cuts/stretch/colormap was only tested with explicit image labels. In normal interactive use no label is passed, so everything lives under the API's default (None) label. This test exercises that path and fails today: the second load resets the cuts, stretch and colormap to the widget defaults. Co-written with Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_015i9rprG92mvoK4JbLKZByA
load_image captured the current cuts/stretch/colormap only when the settings label was not None, but the label is legitimately None in the common case where the caller never passes one (every image then lives under the API's default None key). In that case the capture fell through to the widget defaults, so loading a second image reset the settings. Guard the capture on whether an image is currently displayed (self._data is not None) rather than on the label's value, so a None-keyed current image's settings are carried forward like any other. Co-written with Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_015i9rprG92mvoK4JbLKZByA
The image mark and the two scales are separate widgets, each syncing its own state message the front end redraws on. Today the scales sync first (they release in reverse of their entry order in _hold_all_sync), so the front end draws the old image against the new scales before the new image data arrives -- the visible "refit to window" flash. This test pins the required order and fails today. Co-written with Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_015i9rprG92mvoK4JbLKZByA
_hold_all_sync batched the image mark and both scales, but since they are separate widgets each still syncs its own message and the front end redraws on each. The scales released before the image mark, so the front end drew the old image against the new scales -- the visible "refit to window" flash -- before the new image arrived. Enter the holds in reverse of the desired flush order so the image mark flushes first, then the color scale, then the scales; every intermediate frame then shows the new image. Also fold the color scale into the batch so a colormap change during a load can no longer sync out of band. Co-written with Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_015i9rprG92mvoK4JbLKZByA
There was a problem hiding this comment.
Pull request overview
This PR improves the bqplot-based image viewer implementation to use more sensible display defaults, preserve per-image display settings across updates, and reduce front-end flicker by batching widget syncs during image loads.
Changes:
- Update viewer defaults (cuts interval and colormap) and ensure first render uses stored cuts rather than min/max scaling.
- Preserve cuts/stretch/colormap across
set_cuts/set_stretchand acrossload_imagecalls, including for unlabeled (None) images. - Batch traitlet sync messages during image updates to avoid flicker; add tests and
pytest-mocktest dependency.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
setup.cfg |
Adds pytest-mock to test extras to support new sync-order/batching tests. |
astrowidgets/tests/test_widget_api_bqplot.py |
Adds regression tests for default cuts/colormap, setting persistence, and front-end sync batching/order. |
astrowidgets/bqplot.py |
Implements sync batching, new defaults, and refresh/defer logic; adjusts load_image behavior to preserve settings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mwcraig
commented
Jul 7, 2026
After a bare load the stored cuts are the widget defaults, so the test could not tell a refresh that used the stored cuts from one that fell back to the defaults. Co-written with Claude Fable 5 Claude-Session: https://claude.ai/code/session_013c7zdgS4kAuWzJxmqtcE6X
With the default cuts the check also passed if the batched refresh fell back to the widget defaults instead of the carried settings. Co-written with Claude Fable 5 Claude-Session: https://claude.ai/code/session_013c7zdgS4kAuWzJxmqtcE6X
Pin down what an unlabeled load does once labeled images exist: the API layer resolves the None label to the single user label, so the load replaces the labeled image, keeps that label's settings, leaves the image stored under None untouched, and raises when two or more labels make the target ambiguous. Co-written with Claude Fable 5 Claude-Session: https://claude.ai/code/session_013c7zdgS4kAuWzJxmqtcE6X
Co-written with Claude Fable 5 Claude-Session: https://claude.ai/code/session_013c7zdgS4kAuWzJxmqtcE6X
The API layer keeps settings for the None label even before an image is loaded, so the public method works here and additionally stores the default where get_colormap can report it. Co-written with Claude Fable 5 Claude-Session: https://claude.ai/code/session_013c7zdgS4kAuWzJxmqtcE6X
Member
|
Just merge when you ready. Thanks! |
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.
It turns out the bqplot had a number of issues:
None-- which is the label used when the user has not specified one -- was not handled correctly.This PR fixes all of those things. The number of commits is large to ensure that each change is bite-sized and easy to review.