Fix bqplot some glitches in the bqplot implementation with the new API#202
Merged
Conversation
The bqplot GUI-change observer only watched the x scale's width and synced fov, so interactive panning (which moves the scales without changing their width) never updated the stored viewport center. A vertical pan was missed entirely since only the x scale was observed. Generalize the observer to read the live center and width and sync both into the stored viewport, and observe the max of both the x and y scales so horizontal pan, vertical pan, and zoom are all captured. Co-written with Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_014WXzFppCtXGddJv6rVgy68
995722e to
0a03017
Compare
set_cuts and set_stretch routed through _send_data, which defaults to reset_view=True and so called reset_scale_to_fit_image -- refitting the whole image to the display every time the cuts or stretch changed. Adjusting the color mapping should not move the viewport. Pass reset_view=False from both methods so only the color mapping changes. load_image still uses the reset_view=True default so a freshly loaded image is fit to the display. Co-written with Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_014WXzFppCtXGddJv6rVgy68 Bump minimum AIDA version
0a03017 to
cadb68b
Compare
There was a problem hiding this comment.
Pull request overview
This PR addresses bqplot backend viewport inconsistencies with the newer astro-image-display-api, ensuring that color-mapping changes (stretch/cuts) don’t inadvertently reset pan/zoom, and that get_viewport() reflects interactive panning done in the browser.
Changes:
- Track interactive pan/zoom by observing bqplot scale changes and syncing the stored viewport accordingly.
- Prevent viewport reset when applying stretch or cuts by avoiding
reset_viewduring data re-send. - Update the bqplot API test import path and add a regression test for interactive pan; add a new bqplot widget example notebook.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| setup.cfg | Bumps minimum astro-image-display-api version to support the new API surface used by tests/docs. |
| astrowidgets/bqplot.py | Syncs stored viewport with interactive pan/zoom and avoids resetting viewport on stretch/cuts changes. |
| astrowidgets/tests/test_widget_api_bqplot.py | Updates ImageAPITest import and adds an interactive-pan viewport regression test. |
| example_notebooks/bqplot_widget.ipynb | Adds a bqplot widget example notebook demonstrating the updated API usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ok typos
Initialize self._updating_viewport in __init__ so the scale observer's
re-entrancy guard is always defined, and fix two typos in the bqplot
example notebook ("witht he" -> "with the", "Maker" -> "Marker").
Co-written with Claude Opus 4.8
Member
|
Who is affected by this? Should someone from Jdaviz review? I am not sure if I have opinions on this API. |
Member
Author
|
I don't think anyone except me is affected by this, tbh....as far as I know I am the only user ever of the bqplot implementation. I think the jdaviz folks are writing/have written their own AIDA-compliant viewer in jdaviz. Happy to ping someone though -- usggestion? |
Member
|
In that case... merge when you want? |
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.
There were a couple places where the bqplot widget gave some unexpected results: