Skip to content

Update ginga viewer to match AIDA interface#205

Draft
mwcraig wants to merge 14 commits into
astropy:mainfrom
mwcraig:update-ginga-aida
Draft

Update ginga viewer to match AIDA interface#205
mwcraig wants to merge 14 commits into
astropy:mainfrom
mwcraig:update-ginga-aida

Conversation

@mwcraig

@mwcraig mwcraig commented Jul 4, 2026

Copy link
Copy Markdown
Member

This pull request updates the ginga viewer to use the astro-image-display-api. The commit history is a little noisy -- I was using AI to help update the code and wanted small commits I could more easily review -- but I'm happy to squash before merging.

This also contains the broken import that appears in #204, so once this is merged a rebase of #204 should fix that one.

mwcraig added 12 commits July 4, 2026 09:32
Bring the ginga backend up to the same astro-image-display-api (AIDA)
standard as the bqplot backend. The widget now inherits ImageViewerLogic
and exposes only the AIDA API publicly, plus the interactive extras that
ginga supports natively.

- Replace the broken ImageViewerInterface.* class attributes (which no
  longer exist and prevented import) with plain literals.
- load_image stores data/WCS via super(), builds a ginga AstroImage, and
  re-applies the stored viewport, cuts, stretch and colormap.
- set_cuts/set_stretch map astropy objects onto ginga cut levels and color
  algorithms without disturbing the viewport; set_colormap maps the name.
- Catalog API draws on the ginga canvas under a tag = str(catalog_label).
- Viewport API converts the pixel field of view to a ginga scale as
  scale = window / fov; get_viewport syncs live ginga pan/zoom into the
  stored viewport (only when the user actually interacted, keeping
  programmatic round-trips exact).
- Keep interactive extras: cursor, click_center, click_drag, scroll_pan,
  print_out, image_width/height, and interactive marking wired to the
  catalog API.
- Drop the ginga-native public methods (center_on, offset_by, zoom,
  add_markers, marker, cuts/stretch properties, ...).
- Fix the save() existence check (Path(filename).exists()).

Passes the AIDA compliance suite (56 passed, 2 save tests skipped),
matching the bqplot backend.

Co-written with Claude Opus 4.8
Rebuild example_notebooks/ginga_widget.ipynb around the AIDA API, mirroring
bqplot_widget.ipynb cell-for-cell for the shared sections (load_image,
set/get_viewport with pixel/SkyCoord/angular fov, cuts/stretch, colormap,
catalog load/style/remove including a use_skycoord catalog and the
ipywidgets slider demo, get_image/save).

Because ginga has a real WCS, the viewport and catalog cells run for real
rather than as placeholders. The interactive tail is replaced with working
ginga-native demos -- the key-bindings table, live cursor bar, click_center,
and start_marking/stop_marking/is_marking with the points retrieved via
get_catalog -- instead of bqplot's "not available" notes.

Imports now use `from astrowidgets.ginga import ImageWidget`. Verified by
executing the notebook headless with no cell errors.

Co-written with Claude Opus 4.8
Use ImageViewerLogic.__post_init__ to set up dataclass state instead of
hand-rolling the call to _set_up_catalog_image_dicts, drop the stale
ipyevents comment, and remove the redundant image_label resolution in
load_image (the helpers resolve it internally).

Co-written with Claude Opus 4.8
Pass extra keyword arguments through to ipywidgets.VBox so custom widget
traits (e.g. layout) are honored. The deprecated use_opencv kwarg is
popped first so it still warns rather than reaching VBox as an unknown
trait.

Co-written with Claude Opus 4.8
Drop the pixel_coords_offset constructor argument, the pixel_offset
property, and the associated offset arithmetic in the mouse callbacks and
_center_on. The API-approved way to shift coordinates is through the
viewport, so this special-case offset is no longer needed.

Co-written with Claude Opus 4.8
Replace the direct ginga set_pan calls in _center_on with a call to the
public set_viewport, so all re-centering goes through the AIDA viewport
bookkeeping (including the WCS handling for SkyCoord arguments). Behavior
is unchanged; the new tests are regression coverage. The now-unused
SkyCoord import is dropped.

Co-written with Claude Opus 4.8
Drop the image_width/image_height setters; the API-approved way to resize
the view is through the viewport. The getters are kept because they are
used by _viewport_window_size and set_window_size.

Co-written with Claude Opus 4.8
Use the ginga viewer's save_rgb_image_as_file renderer instead of writing
out the JPEG-encoded widget buffer. This produces a file in the format
implied by the extension (e.g. a real PNG for a .png filename) and does
not depend on a running browser. The overwrite guard is unchanged.

Co-written with Claude Opus 4.8
Replace the open review questions about cuts limits and the colormap None
guard with the confirmed answers: get_limits is exact for any interval
type, and the colormap legitimately defaults to None.

Co-written with Claude Opus 4.8
The viewport conversion already works for a non-square viewer: ginga
scales isotropically and both set_viewport and get_viewport use the window
width as the single fov reference dimension, so the round-trip is exact
regardless of aspect ratio. Document this in _viewport_window_size and add
a regression test for a non-square viewer.

Co-written with Claude Opus 4.8
Widen the try/except to cover the whole WCS-construction block (header
serialization, ginga's header parser, and set_wcs), any of which can fail
on a malformed or exotic WCS, so a bad WCS degrades gracefully to
displaying the image without one. Keep catching Exception (not
BaseException, so Ctrl-C still propagates) but log the traceback via the
ginga logger instead of printing only the message.

Co-written with Claude Opus 4.8
Ginga's ColorDist classes are parametrized, so a parametrized astropy
stretch can be reproduced instead of losing its shape parameter. Inject a
fully parametrized distribution via rgbmap.set_dist (set_color_algorithm
only ever builds ginga defaults):

- AsinhStretch/SinhStretch map exactly onto AsinhDist/SinhDist via
  factor=1/a, nonlinearity=arcsinh(1/a) (resp. sinh).
- LogStretch/PowerDistStretch configure LogDist/PowerDist with exp=a,
  matching the curve shape to within a quantization level.
- SqrtStretch/SquaredStretch/default LinearStretch use the matching
  parameterless native dist.
- Everything ginga lacks a family for -- PowerStretch (x**a, a different
  family than ginga's 'power' = PowerDistStretch), ContrastBiasStretch,
  HistEqStretch, composites -- is reproduced exactly by a new
  _AstropyStretchDist adapter that evaluates the stretch on ginga's 0..1
  ramp. This removes the warn-and-fall-back-to-linear path.

Tests assert the configured ginga ColorDist's lookup table matches the
astropy stretch on the same ramp, that the shape parameter reaches ginga,
and that PowerStretch is not mis-mapped to ginga's 'power'.

Co-written with Claude Opus 4.8

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request updates the Ginga-backed ImageWidget to implement the astro-image-display-api (AIDA) interface, aligning behavior with the shared viewer API used across backends and updating examples/tests accordingly.

Changes:

  • Refactors astrowidgets.ginga.ImageWidget to integrate ImageViewerLogic and implement AIDA methods (viewport, cuts/stretch/colormap, catalogs).
  • Adds a comprehensive Ginga-specific AIDA test suite and removes the prior minimal/legacy test stub.
  • Updates the Ginga example notebook to use the AIDA API methods and separates “AIDA API” vs “Ginga-native” interactions.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
astrowidgets/ginga.py Major refactor to AIDA-compliant implementation + Ginga-specific adapters for stretch/cuts/catalog rendering.
astrowidgets/tests/test_widget_api_ginga.py New AIDA conformance + Ginga-behavior tests for viewport and stretch mapping.
astrowidgets/tests/_test_widget_api_ginga.py Removes old minimal test file superseded by the new suite.
example_notebooks/ginga_widget.ipynb Updates notebook examples to use AIDA API methods and documents Ginga-native interactivity separately.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread astrowidgets/ginga.py
Comment on lines 159 to 166
if 'use_opencv' in kwargs:
# Pop it so it is not forwarded to VBox, which would raise a
# TraitError for an unknown trait.
kwargs.pop('use_opencv')
warnings.warn("use_opencv kwarg has been deprecated--"
"opencv will be used if it is installed",
DeprecationWarning)

Comment thread astrowidgets/ginga.py
Comment on lines +455 to +457
shape = style.get('shape', 'circle')
color = style.get('color', 'red')
size = style.get('size', 5)
"ra_str = '01h13m23.193s'\n",
"dec_str = '+00d12m32.19s'\n",
"frame = 'icrs'\n",
"frame = 'galactic'\n",
Comment on lines +616 to +620
" try:\n",
" w.remove_catalog(catalog_label='stars')\n",
" except ValueError as exc:\n",
" raise exc # Catalog doesn't exist\n",
" \n",
@pllim

pllim commented Jul 6, 2026

Copy link
Copy Markdown
Member

I guess you and the AI better hash things out first before I approve?

@mwcraig

mwcraig commented Jul 6, 2026

Copy link
Copy Markdown
Member Author

Good call -- turns out that the bqplot viewer that I thought was done is definitely not done...lesson being do more local tests in a browser before pushing.

@mwcraig mwcraig marked this pull request as draft July 8, 2026 14:26
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.

3 participants