glue-viz / glue

Linked Data Visualizations Across Multiple Files
http://glueviz.org
Other
742 stars 153 forks source link

Scatter Layer on Image Viewer breaks export-as-script #2457

Open jfoster17 opened 1 year ago

jfoster17 commented 1 year ago

Describe the bug The "Save Python script..." option fails for an ImageViewer with a Scatter layer on top.

To Reproduce Steps to reproduce the behavior such as:

  1. Create an ImageViewer and add some points on top (e.g. with the W5 sample data)
  2. Use the "Save" toolbar to "Save Python script to reproduce plot"
  3. Nothing apparently happens, but an error message is thrown
Traceback (most recent call last):
  File "/Users/jfoster/Developer/glue-genes-with-mainline-glue/glue-qt/glue_qt/viewers/common/toolbar.py", line 145, in trigger
    self.active_tool = tool
  File "/Users/jfoster/Developer/glue-genes-with-mainline-glue/glue-qt/glue_qt/viewers/common/toolbar.py", line 79, in active_tool
    self.activate_tool(new_tool)
  File "/Users/jfoster/Developer/glue-genes-with-mainline-glue/glue-qt/glue_qt/viewers/common/toolbar.py", line 98, in activate_tool
    tool.activate()
  File "/Users/jfoster/Developer/glue-genes-with-mainline-glue/glue/glue/plugins/tools/python_export.py", line 24, in activate
    self.viewer.export_as_script(filename)
  File "/Users/jfoster/Developer/glue-genes-with-mainline-glue/glue/glue/utils/matplotlib.py", line 180, in wrapper
    result = func(*args, **kwargs)
  File "/Users/jfoster/Developer/glue-genes-with-mainline-glue/glue/glue/utils/matplotlib.py", line 175, in wrapper
    return func(*args, **kwargs)
  File "/Users/jfoster/Developer/glue-genes-with-mainline-glue/glue/glue/viewers/common/viewer.py", line 478, in export_as_script
    imports_layer, layer_script = layer._python_exporter(layer)
  File "/Users/jfoster/Developer/glue-genes-with-mainline-glue/glue/glue/viewers/scatter/python_export.py", line 13, in python_export_scatter_layer
    polar = layer._viewer_state.using_polar
AttributeError: 'ImageViewerState' object has no attribute 'using_polar'

Details:

Additional context I'm not sure when/if this previously worked. It seems like much of the logic in python_export_scatter_layer assumes that we are exporting a standalone ScatterViewer (many calls to layer._viewer_state). The hacky fix would be to disable this option for an image viewer with additional non-image layers on top.

jfoster17 commented 1 year ago

NB -- this will also apply to RegionScatterLayers (once they are implemented). We can fix both at once.

Carifio24 commented 1 year ago

I would guess that changing the offending line to polar = getattr(layer._viewer_state, 'using_polar', False) should be enough to fix this, but I admittedly haven't tried it.

jfoster17 commented 1 year ago

That would deal with this particular error, but there are other calls to layer._viewer_state in this to fix.

Carifio24 commented 1 year ago

Ah yeah, looks like there are similar calls for other scatter viewer state properties (full sphere, degrees)

Carifio24 commented 12 months ago

I don't know when exactly this broke, but this is clearly something that I didn't check on when I added the support for the full-sphere and angle unit functionality. I can confirm that doing e.g.

polar = getattr(layer._viewer_state, 'using_polar', False)
full_sphere = getattr(layer._viewer_state, 'using_full_sphere', False)
degrees = getattr(layer._viewer_state, 'using_degrees', False)

will make things work for a scatter layer in an image viewer.

I'm not sure what adding support for region layers would entail - if the behavior for those is significantly different we could use a dispatch function for the layer export. Not ideal, but we do need some kind of check on the viewer state so we can properly handle any pretransforms that the exporting viewer is using.