diff --git a/astrowidgets/bqplot.py b/astrowidgets/bqplot.py index e8d1fdc..a06d6f5 100644 --- a/astrowidgets/bqplot.py +++ b/astrowidgets/bqplot.py @@ -1,3 +1,4 @@ +from contextlib import contextmanager from pathlib import Path import numpy as np @@ -157,19 +158,41 @@ def _set_scale_aspect_ratio_to_match_viewer(self, self._scales[reset_scale].max = frozen_width[reset_scale] * scale_factor self.center = current_center + @contextmanager + def _hold_all_sync(self): + """ + Batch trait sync messages for the image mark, its color scale and + both axis scales so that the front end receives a single state + update per widget, and hence redraws once, instead of redrawing + after every trait assignment. + + These are separate widgets, so each still syncs its own message and + the front end redraws on each. On exit the contexts release -- and + each widget flushes -- in reverse of the order entered here, so the + entry order is chosen to make the image mark flush first, then the + color scale, then the scales. That way the front end never draws + the old image against the new scales (a "refit" flash) nor recolors + the old image before the new data arrives. + """ + with self._scales['y'].hold_sync(), self._scales['x'].hold_sync(), \ + self._image.scales['image'].hold_sync(), \ + self._image.hold_sync(): + yield + def set_data(self, image_data, reset_view=True): self._image_shape = image_data.shape - if reset_view: - self.reset_scale_to_fit_image() + with self._hold_all_sync(): + if reset_view: + self.reset_scale_to_fit_image() - # Set the image data and map it to the bqplot figure so that - # cursor location corresponds to the underlying array index. - # The offset follows the convention that the index corresponds - # to the center of the pixel. - self._image.image = image_data - self._image.x = [-0.5, self._image_shape[1] - 0.5] - self._image.y = [-0.5, self._image_shape[0] - 0.5] + # Set the image data and map it to the bqplot figure so that + # cursor location corresponds to the underlying array index. + # The offset follows the convention that the index corresponds + # to the center of the pixel. + self._image.image = image_data + self._image.x = [-0.5, self._image_shape[1] - 0.5] + self._image.y = [-0.5, self._image_shape[0] - 0.5] @property def scale_widths(self): @@ -364,8 +387,15 @@ def __init__(self, *args, display_width=500, display_aspect_ratio=1): self._astro_im = _AstroImage(display_width=display_width, viewer_aspect_ratio=display_aspect_ratio) - self._default_cuts = apviz.MinMaxInterval() + # Cut out the sky background at the bottom and clip only the + # brightest pixels at the top. + self._default_cuts = apviz.AsymmetricPercentileInterval(30, 96) self._default_stretch = None + self._default_colormap = 'Greys_r' + # Store the default through the API layer (which keeps settings + # for the None label even before a load) so that get_colormap + # reports it; set_colormap also applies it to the front end. + self.set_colormap(self._default_colormap) self._data = None self._wcs = None @@ -377,12 +407,14 @@ def __init__(self, *args, display_width=500, display_aspect_ratio=1): # Guards re-entrancy while we programmatically update the viewport. self._updating_viewport = False + # While True, _refresh_display does nothing; set by _defer_refresh. + self._refresh_deferred = False + # Provide an Output widget to which prints can be directed for # debugging. self._print_out = ipw.Output() self.marker = {'color': 'red', 'radius': 20, 'type': 'square'} - self._cuts = apviz.AsymmetricPercentileInterval(1, 99) self._cursor = ipw.HTML('Coordinates show up here') @@ -539,6 +571,31 @@ def _send_data(self, reset_view=True, stretch=None, cuts=None): self._astro_im.set_data(self._interval_and_stretch(stretch=stretch, cuts=cuts), reset_view=reset_view) + def _refresh_display(self, image_label=None, reset_view=False): + """ + Recompute the displayed array from the cuts and stretch stored + for the image and send it to the viewer. + """ + if self._data is None or self._refresh_deferred: + return + + self._send_data(cuts=self.get_cuts(image_label=image_label), + stretch=self.get_stretch(image_label=image_label), + reset_view=reset_view) + + @contextmanager + def _defer_refresh(self): + """ + Make _refresh_display a no-op inside the block so that several + settings can be stored with a single recomputation of the + displayed array. The caller refreshes after the block. + """ + self._refresh_deferred = True + try: + yield + finally: + self._refresh_deferred = False + @property def _current_image_label(self): """ @@ -550,14 +607,13 @@ def set_stretch(self, value, image_label=None, **kwargs): super().set_stretch(value, image_label=image_label, **kwargs) # Changing the stretch only affects the color mapping, so leave the # current viewport (zoom/pan) untouched. - self._send_data(stretch=value, reset_view=False) + self._refresh_display(image_label=image_label) def set_cuts(self, value, image_label=None, **kwargs): super().set_cuts(value, image_label=image_label, **kwargs) # Changing the cuts only affects the color mapping, so leave the # current viewport (zoom/pan) untouched. - self._send_data(cuts=self.get_cuts(image_label=image_label), - reset_view=False) + self._refresh_display(image_label=image_label) @property def viewer(self): @@ -565,11 +621,54 @@ def viewer(self): # The methods, grouped loosely by purpose def load_image(self, image, image_label=None, **kwargs): - super().load_image(image, image_label=image_label, **kwargs) - data = self.get_image(image_label=image_label) + # A newly loaded image is always displayed with the current cuts, + # stretch and colormap; only the viewport resets. Capture the + # current settings before the API layer replaces them with its own + # defaults during the load. + if image_label is not None and image_label in self._images: + # Re-loading an existing image: its own settings are current. + settings_label = image_label + have_current = True + elif self._data is not None: + # A new image: carry forward from the displayed image. Its label + # is legitimately None when the caller never passed one, so guard + # on whether an image is displayed, not on the label's value. + settings_label = self._current_image_label + have_current = True + else: + # Nothing has been displayed yet. + have_current = False - self._data = data.data if isinstance(data, NDData) else data - self._send_data() + if have_current: + current_cuts = self.get_cuts(image_label=settings_label) + current_stretch = self.get_stretch(image_label=settings_label) + current_colormap = self.get_colormap(image_label=settings_label) + else: + current_cuts = self._default_cuts + # Leave the stretch the API layer stores on load (linear). + current_stretch = self._default_stretch + current_colormap = self._default_colormap + + # Hold the sync so the scale changes from the viewport + # initialization in the API layer arrive at the front end in the + # same batch as the new image data, avoiding flicker from + # intermediate redraws. + with self._astro_im._hold_all_sync(): + # Store the settings for the new image so the get_* methods + # report what is displayed. Defer the refresh each setter + # triggers; one refresh at the end displays the image. + with self._defer_refresh(): + super().load_image(image, image_label=image_label, **kwargs) + + self.set_cuts(current_cuts, image_label=image_label) + if current_stretch is not None: + self.set_stretch(current_stretch, image_label=image_label) + self.set_colormap(current_colormap, image_label=image_label) + + data = self.get_image(image_label=image_label) + self._data = data.data if isinstance(data, NDData) else data + + self._refresh_display(image_label=image_label, reset_view=True) # Saving contents of the view and accessing the view def save(self, filename, overwrite=False, **kwargs): diff --git a/astrowidgets/tests/test_widget_api_bqplot.py b/astrowidgets/tests/test_widget_api_bqplot.py index baf23ff..0f46167 100644 --- a/astrowidgets/tests/test_widget_api_bqplot.py +++ b/astrowidgets/tests/test_widget_api_bqplot.py @@ -1,3 +1,5 @@ +import astropy.visualization as apviz +import numpy as np import pytest from traitlets import TraitError @@ -8,7 +10,7 @@ _ = pytest.importorskip("bqplot", reason="Package required for test is not " "available.") -from astrowidgets.bqplot import ImageWidget # noqa: E402 +from astrowidgets.bqplot import ImageWidget, bqcolors # noqa: E402 def test_instance(): @@ -30,6 +32,301 @@ def test_save(self, tmp_path): def test_save_overwrite(self, tmp_path): pass + def test_initial_display_uses_stored_cuts(self): + # A high-dynamic-range image: narrow uint16 background plus a single + # saturated pixel. Scaling to the full data range would render the + # background essentially black, so the initial display must use the + # stored cuts (a percentile interval), not min/max. + rng = np.random.default_rng(seed=42) + arr = rng.integers(1100, 1300, size=(50, 60)).astype(np.uint16) + arr[25, 30] = 65535 + + self.image.load_image(arr) + + displayed = np.asarray(self.image._astro_im._image.image) + expected = self.image.get_cuts()(arr) + np.testing.assert_allclose(displayed, expected) + + def test_default_cuts_are_30_96_percentile(self): + # When no cuts are stored or passed, the widget's fallback should + # be the 30-96 percentile interval, which cuts out the sky + # background and clips only the brightest pixels. + rng = np.random.default_rng(seed=42) + arr = rng.integers(1100, 1300, size=(50, 60)).astype(np.uint16) + arr[25, 30] = 65535 + + self.image._data = arr + expected = apviz.AsymmetricPercentileInterval(30, 96)(arr) + np.testing.assert_allclose(self.image._interval_and_stretch(), expected) + + def test_default_colormap_is_greys_r(self, data): + # With no colormap explicitly set, the display should use Greys_r + # (low = black, high = white), the usual astronomical convention, + # both before and after an image is loaded, and get_colormap should + # report it. + greys_r = bqcolors('Greys_r') + assert self.image._astro_im._image.scales['image'].colors == greys_r + assert self.image.get_colormap() == 'Greys_r' + + self.image.load_image(data) + assert self.image.get_colormap() == 'Greys_r' + assert self.image._astro_im._image.scales['image'].colors == greys_r + + def test_load_image_keeps_current_display_settings(self): + # Loading a new image should display it with the cuts, stretch and + # colormap that are currently in effect, carried forward from the + # previously displayed image, and store them for the new image so + # that the get_* methods agree with the display. Only the viewport + # resets on load. + rng = np.random.default_rng(seed=42) + arr = rng.integers(1100, 1300, size=(50, 60)).astype(np.uint16) + arr[25, 30] = 65535 + + self.image.load_image(arr, image_label='first') + cuts = apviz.AsymmetricPercentileInterval(5, 90) + stretch = apviz.LogStretch() + self.image.set_cuts(cuts, image_label='first') + self.image.set_stretch(stretch, image_label='first') + self.image.set_colormap('viridis', image_label='first') + + self.image.load_image(arr, image_label='second') + + assert self.image.get_cuts(image_label='second') is cuts + assert self.image.get_stretch(image_label='second') is stretch + assert self.image.get_colormap(image_label='second') == 'viridis' + assert self.image._astro_im._image.scales['image'].colors == bqcolors('viridis') + + displayed = np.asarray(self.image._astro_im._image.image) + np.testing.assert_allclose(displayed, stretch(cuts(arr))) + + def test_reload_existing_label_keeps_its_settings(self): + # When new data is loaded under an existing image label, that + # label's own stored settings are the current ones for the image + # and must be kept, even if a different image was loaded (and so + # displayed) more recently. + rng = np.random.default_rng(seed=42) + arr = rng.integers(1100, 1300, size=(50, 60)).astype(np.uint16) + arr[25, 30] = 65535 + + first_cuts = apviz.AsymmetricPercentileInterval(5, 90) + first_stretch = apviz.LogStretch() + self.image.load_image(arr, image_label='first') + self.image.set_cuts(first_cuts, image_label='first') + self.image.set_stretch(first_stretch, image_label='first') + self.image.set_colormap('viridis', image_label='first') + + self.image.load_image(arr, image_label='second') + self.image.set_cuts(apviz.ManualInterval(1100, 1300), + image_label='second') + self.image.set_stretch(apviz.SqrtStretch(), image_label='second') + self.image.set_colormap('plasma', image_label='second') + + new_arr = arr + 10 + self.image.load_image(new_arr, image_label='first') + + assert self.image.get_cuts(image_label='first') is first_cuts + assert self.image.get_stretch(image_label='first') is first_stretch + assert self.image.get_colormap(image_label='first') == 'viridis' + assert self.image._astro_im._image.scales['image'].colors == bqcolors('viridis') + + displayed = np.asarray(self.image._astro_im._image.image) + np.testing.assert_allclose(displayed, first_stretch(first_cuts(new_arr))) + + def test_load_image_keeps_settings_without_labels(self): + # The carry-forward of cuts, stretch and colormap must work in the + # common interactive case where no image_label is ever passed, so + # every image and its settings live under the API's default (None) + # label. Loading a second image must keep the settings currently in + # effect, not fall back to the widget defaults. + rng = np.random.default_rng(seed=42) + arr = rng.integers(1100, 1300, size=(50, 60)).astype(np.uint16) + arr[25, 30] = 65535 + + self.image.load_image(arr) + cuts = apviz.AsymmetricPercentileInterval(5, 90) + stretch = apviz.LogStretch() + self.image.set_cuts(cuts) + self.image.set_stretch(stretch) + self.image.set_colormap('viridis') + + # A differently shaped second image, still with no label. + arr2 = rng.integers(1100, 1300, size=(40, 30)).astype(np.uint16) + arr2[10, 15] = 65535 + self.image.load_image(arr2) + + assert self.image.get_cuts() is cuts + assert self.image.get_stretch() is stretch + assert self.image.get_colormap() == 'viridis' + assert self.image._astro_im._image.scales['image'].colors == bqcolors('viridis') + + displayed = np.asarray(self.image._astro_im._image.image) + np.testing.assert_allclose(displayed, stretch(cuts(arr2))) + + def test_load_image_without_label_targets_single_labeled_image(self): + # An image label of None means "the caller did not specify a + # label". Once a labeled image exists, the API layer resolves None + # to that label, so an unlabeled load replaces the labeled image + # (keeping its settings) and never touches an image stored under + # the None label. With two or more labels the target is ambiguous + # and the load raises. + rng = np.random.default_rng(seed=42) + arr = rng.integers(1100, 1300, size=(50, 60)).astype(np.uint16) + arr[25, 30] = 65535 + + unlabeled_cuts = apviz.AsymmetricPercentileInterval(5, 90) + self.image.load_image(arr) + self.image.set_cuts(unlabeled_cuts) + + labeled_cuts = apviz.ManualInterval(1100, 1300) + self.image.load_image(arr + 1, image_label='labeled') + self.image.set_cuts(labeled_cuts, image_label='labeled') + + self.image.load_image(arr + 2) + + # The unlabeled load replaced the labeled image and kept its cuts. + np.testing.assert_array_equal( + np.asarray(self.image.get_image(image_label='labeled')), arr + 2) + assert self.image.get_cuts(image_label='labeled') is labeled_cuts + + # The image stored under the None label kept its own settings. + # Once a user label exists, get_cuts(image_label=None) resolves to + # that label, so look at the stored settings directly. + assert self.image._images[None].cuts is unlabeled_cuts + + # With two user-defined labels an unlabeled load is ambiguous. + self.image.load_image(arr, image_label='labeled2') + with pytest.raises(ValueError, match='Multiple image labels'): + self.image.load_image(arr) + + def test_first_load_stores_widget_default_cuts(self): + # With nothing loaded yet there are no current settings to carry + # forward, so the first image should be displayed with the widget's + # default cuts, and those cuts should be stored so that get_cuts + # reports what is displayed. + rng = np.random.default_rng(seed=42) + arr = rng.integers(1100, 1300, size=(50, 60)).astype(np.uint16) + arr[25, 30] = 65535 + + self.image.load_image(arr) + + cuts = self.image.get_cuts() + assert isinstance(cuts, apviz.AsymmetricPercentileInterval) + assert cuts.lower_percentile == 30 + assert cuts.upper_percentile == 96 + + displayed = np.asarray(self.image._astro_im._image.image) + np.testing.assert_allclose(displayed, cuts(arr)) + + def test_set_stretch_keeps_stored_cuts(self): + # Changing the stretch must re-display using the cuts stored for + # the image, not fall back to the widget's default cuts. + rng = np.random.default_rng(seed=42) + arr = rng.integers(1100, 1300, size=(50, 60)).astype(np.uint16) + arr[25, 30] = 65535 + + self.image.load_image(arr) + # Cuts that differ from the widget default, so that a refresh + # falling back to the default cuts produces a different array. + cuts = apviz.AsymmetricPercentileInterval(5, 90) + self.image.set_cuts(cuts) + stretch = apviz.LogStretch() + self.image.set_stretch(stretch) + + displayed = np.asarray(self.image._astro_im._image.image) + np.testing.assert_allclose(displayed, stretch(cuts(arr))) + + def test_set_cuts_keeps_stored_stretch(self): + # Changing the cuts must re-display using the stretch stored for + # the image, not fall back to the widget's default stretch. + rng = np.random.default_rng(seed=42) + arr = rng.integers(1100, 1300, size=(50, 60)).astype(np.uint16) + arr[25, 30] = 65535 + + self.image.load_image(arr) + stretch = apviz.LogStretch() + self.image.set_stretch(stretch) + cuts = apviz.AsymmetricPercentileInterval(5, 90) + self.image.set_cuts(cuts) + + displayed = np.asarray(self.image._astro_im._image.image) + expected = stretch(cuts(arr)) + np.testing.assert_allclose(displayed, expected) + + def test_load_image_batches_frontend_updates(self, data, mocker): + # Every traitlet assignment is normally synced to the browser as + # its own message, each triggering a redraw, so loading an image + # produced a series of visible intermediate states (flicker). + # Loading should batch the updates so the image mark and each + # scale send at most one state message. + self.image.load_image(data, image_label='first') + # Cuts that differ from the widget default, so the end-state check + # below can tell the carried-forward settings from a fallback to + # the defaults. + cuts = apviz.AsymmetricPercentileInterval(5, 90) + self.image.set_cuts(cuts, image_label='first') + + astro_im = self.image._astro_im + image_mark = astro_im._image + scale_x = astro_im._scales['x'] + scale_y = astro_im._scales['y'] + + # Use a different shape so the image extent and scales all change. + arr = np.arange(30 * 40, dtype=float).reshape(30, 40) + + spy_image = mocker.spy(image_mark, 'send_state') + spy_x = mocker.spy(scale_x, 'send_state') + spy_y = mocker.spy(scale_y, 'send_state') + + self.image.load_image(arr, image_label='second') + + assert spy_image.call_count <= 1 + assert spy_x.call_count <= 1 + assert spy_y.call_count <= 1 + + # Batching must not change the end state. + assert self.image.get_cuts(image_label='second') is cuts + displayed = np.asarray(image_mark.image) + np.testing.assert_allclose(displayed, cuts(arr)) + np.testing.assert_allclose(image_mark.x, [-0.5, arr.shape[1] - 0.5]) + np.testing.assert_allclose(image_mark.y, [-0.5, arr.shape[0] - 0.5]) + + def test_load_image_flushes_image_before_scales(self, data, mocker): + # The image mark and the two scales are separate widgets, each + # syncing its own state message that the front end redraws on. + # If a scale syncs before the image mark, the front end briefly + # draws the OLD image against the NEW scales (a visible refit). + # The new image data must reach the front end first. + self.image.load_image(data, image_label='first') + + astro_im = self.image._astro_im + image_mark = astro_im._image + scale_x = astro_im._scales['x'] + scale_y = astro_im._scales['y'] + + # A different shape so the image extent and both scales change. + arr = np.arange(30 * 40, dtype=float).reshape(30, 40) + + order = [] + + def record(name, widget): + real = widget.send_state + + def wrapper(*args, **kwargs): + order.append(name) + return real(*args, **kwargs) + + mocker.patch.object(widget, 'send_state', side_effect=wrapper) + + record('image', image_mark) + record('scale_x', scale_x) + record('scale_y', scale_y) + + self.image.load_image(arr, image_label='second') + + assert 'image' in order + assert order.index('image') < order.index('scale_x') + assert order.index('image') < order.index('scale_y') + def test_get_viewport_reflects_interactive_pan(self, data): # Panning in the browser shifts the bqplot scales directly. Simulate # that here and check that get_viewport reports the new center. diff --git a/setup.cfg b/setup.cfg index 629afc0..70c12cc 100644 --- a/setup.cfg +++ b/setup.cfg @@ -53,6 +53,7 @@ python_requires >=3.10 test = pytest-astropy pytest-cov + pytest-mock docs = sphinx-astropy