EXP: glupyter implementation on abstract class #131

pllim · 2021-02-24T00:16:12Z

This builds on #126 (the abstract class) to implement an additional backend using glue-jupyter and bqplot-image-gl.

Most of the changes are from #126 (they will go away once that PR is merged). The meat of this PR is in:

astrowidgets/glupyter.py
example_notebooks/glupyter/

Known limitations:

Glue toolbar cannot be displayed because ipywidgets.VBox only accept basic widgets as children.
Glue data collection needs to be cleared before loading new data into the same viewer or APIs like w.stretch = 'log' won't work properly (no error but nothing happens on display).

References:

TODO

Separated out the Ginga implementation. Re-organized and updated tests.

TST: Update test matrix and added a test. DOC: Re-organized doc and notebooks. DOC: Update example notebooks

DOC: Cannot use automodapi for Ginga API because we need to inherit docstring. DOC: Do not display inherited members because too confusing. [ci skip]

astrofrog

All the NotImplementedError things are possible (though for the markers it will require a little bit of code to either keep track of the markers in this class or interface with a glue catalog dataset). Happy to provide examples for various parts of this if needed.

astrofrog · 2021-03-02T16:08:10Z

astrowidgets/glupyter.py

+        self._link_image_to_cb()
+
+    def center_on(self, point):
+        raise NotImplementedError  # FIXME


You can find and set the current limits with viewer.state.x_min, viewer.state.x_max, viewer.state.y_min and viewer.state.y_max so you could use that to compute dx and dy (the field of view) then set e.g. viewer.state.x_min = point[0] - dx/2 and so on if point is in pixel coordinates

Given that the viewer size and display scale is already known, is it sufficient if I just set x_min and y_min and then let glupyter take care of recomputing the rest, or is that not a thing?

I am guessing, it takes x/y_min/max that is out of range? Otherwise, I cannot center on edges.

The math isn't quite right. It slowly zooms the image out.

Okay. I think it is because I am setting only x_min and y_min and let it auto adjust for the max. Is that not the right way to do this?

astrofrog · 2021-03-02T16:08:44Z

astrowidgets/glupyter.py

+    def center_on(self, point):
+        raise NotImplementedError  # FIXME
+
+    def offset_to(self, dx, dy, skycoord_offset=False):


Similar to center_on

astrofrog · 2021-03-02T16:09:11Z

astrowidgets/glupyter.py

+        raise NotImplementedError  # FIXME
+
+    @property
+    def zoom_level(self):


you could get x_min/x_max/y_min/y_max as in center_on and compute the fraction of the image shown and return that as zoom level?

I am surprised that zoom info isn't already built into Glue. You have a zoom/pan button after all.

astrofrog · 2021-03-02T16:09:29Z

astrowidgets/glupyter.py

+        raise NotImplementedError  # FIXME
+
+    @zoom_level.setter
+    def zoom_level(self, value):


Again you can do this by setting x_min/x_max/y_min/y_max

astrofrog · 2021-03-02T16:12:33Z

astrowidgets/glupyter.py

+
+    @marker.setter
+    def marker(self, value):
+        raise NotImplementedError  # FIXME


For all the marker stuff you could do it two ways - keep track of the markers in the astrowidgets layer and use the bqplot Scatter class to show the markers (you can add/remove markers by setting the x and y properties on the scatter object). I can show you an example if needed. Another option would be to actually make the list of markers be a glue data object and add it to the image viewer as a normal glue dataset - the advantage of this is that it shows up in the image viewer layers and you can control the appearance of the markers. There is a glue message you can fire if you want to update the glue dataset to force the viewer to refresh so let me know if you decide to go that route.

I think the latter makes more sense. Currently in the Ginga implementation, it creates Ginga canvas objects, and then extracts them back from the Ginga canvas, so this is consistent.

What isn't clear to me is how you control the looks of the Glue markers. For example, how do I ask Glue to draw blue X or red circle? How do I only extract markers that are red circle? How do I extract all the markers irrespective of shape/color?

astrofrog · 2021-03-02T16:12:56Z

astrowidgets/glupyter.py

+
+    def get_markers_by_name(self, marker_name, x_colname='x', y_colname='y',
+                            skycoord_colname='coord'):
+        raise NotImplementedError  # FIXME


Should be possible if the above is implemented (in both options)

astrofrog · 2021-03-02T16:13:09Z

astrowidgets/glupyter.py

+    def add_markers(self, table, x_colname='x', y_colname='y',
+                    skycoord_colname='coord', use_skycoord=False,
+                    marker_name=None):
+        raise NotImplementedError  # FIXME


Also possible

astrofrog · 2021-03-02T16:13:14Z

astrowidgets/glupyter.py

+        raise NotImplementedError  # FIXME
+
+    def remove_markers_by_name(self, marker_name):
+        raise NotImplementedError  # FIXME


Also possible

astrofrog · 2021-03-02T16:13:58Z

astrowidgets/glupyter.py

+
+    @property
+    def autocut_options(self):
+        raise NotImplementedError  # FIXME: Does Glue support this?


I don't understand what this does but you can set the percentile values in glue to automatically determine the levels/cuts in the image viewer. Could you clarify what this should return?

For Ginga, this is what happens:

>>> w.autocut_options ('minmax', 'median', 'histogram', 'stddev', 'zscale') >>> w.cuts = 'zscale' # Automatically sets the cut levels according to this algorithm

But if this does not make sense for Glue (or other future backends), then maybe I'll take autocut_options out of the abstract class and make it a Ginga-only option... 💭

mwcraig · 2021-06-30T16:14:33Z

I'm finally taking a look at this batch of PRs and decided to start with this one.

There are two videos below, one with the implementation in this PR that uses the BqplotImageView from glue-jupyter. The other uses the mouse demo notebook from https://github.com/glue-viz/bqplot-image-gl so it is displaying a bqplot figure using the ImageGL mark. In the glue-baed version there is considerable lag when zooming out or panning.

Is there a way to eliminate the lag in rendering in when zooming out or panning? (maybe an @astrofrog question since the answer is likely in glue)?

glupyter demo

bqplot/ImageGL direct demo

pllim · 2021-06-30T16:21:28Z

Re: lag -- It is the same problem as spacetelescope/jdaviz#564 and @astrofrog said there, "This is going to be harder to fix though we can try and profile it to see why it's slow. Essentially whenever you pan or zoom, we re-compute a fixed resolution buffer and send it to the front-end. We do this because for large images, sending the whole image to the GPU is not feasible. Essentially we behave a little like Google maps does. Having said this, we could imagine computing a buffer that extends beyond the current image limits so that the experience is a little nicer. I would personally advocate for that being a little lower priority compared to other features but it would be good to improve at some point."

pllim · 2021-06-30T16:23:45Z

As for the abstract class, I am not sure how to proceed with this PR. I think @astrofrog disagree with some low level implementations here that would warrant refactor even on Ginga's implementation, so there is a high barrier.

For now, we have decided on a "shim" over at Jdaviz. Also see spacetelescope/jdaviz#631 and related PRs over there.

mwcraig · 2021-06-30T16:55:32Z

As for the abstract class, I am not sure how to proceed with this PR. I think @astrofrog disagree with some low level implementations here that would warrant refactor even on Ginga's implementation, so there is a high barrier.

I might take another shot at it this week -- I have some time through SciPy to devote to it.

I started with this PR because I found it easier to think about the base class after seeing a second implementation of the viewer.

pllim · 2021-06-30T17:18:43Z

@mwcraig , there is also an unresolved comment here: #93 (comment)

pllim · 2023-06-16T15:12:54Z

Most of the work ended up in Jdaviz. This is no longer relevant.

pllim added 9 commits February 19, 2021 11:26

WIP: Initial implementation of abstract class.

7abdf62

Separated out the Ginga implementation. Re-organized and updated tests.

TST: Handle empty warning in astropy#128

d4c0c36

TST: Remove unused select_points test in astropy#127

9800195

DOC: Update user-facing doc.

0a87db8

TST: Update test matrix and added a test. DOC: Re-organized doc and notebooks. DOC: Update example notebooks

TST: Disable oldest deps job

f8082b3

TST: Fix deps for devdeps job

7efa6a4

DOC: Fix inheritance diagram for Ginga API.

678a2e4

DOC: Cannot use automodapi for Ginga API because we need to inherit docstring. DOC: Do not display inherited members because too confusing. [ci skip]

WIP: Building on astropy#126

8f39bd8

WIP: First working draft

f038409

pllim requested review from mwcraig, astrofrog and eteq February 24, 2021 00:16

pllim added the enhancement New feature or request label Feb 24, 2021

Unique ID that is consistent with jdaviz

c594d57

pllim mentioned this pull request Feb 24, 2021

imviz-round-1.5 (astrowidgets + Glupyter) spacetelescope/jdaviz#445

Closed

4 tasks

Fix cursor info panel crashing on click

6e53cd2

astrofrog reviewed Mar 2, 2021

View reviewed changes

pllim mentioned this pull request Mar 2, 2021

Widget should not inherit from VBox #132

Closed

Base automatically changed from master to main March 9, 2021 22:47

pllim mentioned this pull request May 20, 2021

[Meta PR] Imviz: astrowidget API spacetelescope/jdaviz#632

Closed

14 tasks

pllim mentioned this pull request Jun 30, 2021

rename offset_to to offset_byand have it use units #139

Closed

pllim added the backend: glue-jupyter label Jun 16, 2023

pllim closed this Jun 16, 2023

pllim deleted the abstract-class-glupyter branch June 16, 2023 15:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EXP: glupyter implementation on abstract class #131

EXP: glupyter implementation on abstract class #131

pllim commented Feb 24, 2021 •

edited

astrofrog left a comment

astrofrog Mar 2, 2021

pllim Mar 2, 2021

pllim Jun 16, 2021

pllim Jun 16, 2021

astrofrog Mar 2, 2021

astrofrog Mar 2, 2021

pllim Mar 2, 2021

astrofrog Mar 2, 2021

astrofrog Mar 2, 2021

pllim Mar 2, 2021

astrofrog Mar 2, 2021

astrofrog Mar 2, 2021

astrofrog Mar 2, 2021

astrofrog Mar 2, 2021

pllim Mar 2, 2021

mwcraig commented Jun 30, 2021

pllim commented Jun 30, 2021

pllim commented Jun 30, 2021

mwcraig commented Jun 30, 2021

pllim commented Jun 30, 2021

pllim commented Jun 16, 2023

EXP: glupyter implementation on abstract class #131

EXP: glupyter implementation on abstract class #131

Conversation

pllim commented Feb 24, 2021 • edited

TODO

astrofrog left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mwcraig commented Jun 30, 2021

glupyter demo

bqplot/ImageGL direct demo

pllim commented Jun 30, 2021

pllim commented Jun 30, 2021

mwcraig commented Jun 30, 2021

pllim commented Jun 30, 2021

pllim commented Jun 16, 2023

pllim commented Feb 24, 2021 •

edited