Merge pull request #1289 from astrofrog/refactor-scatter-image

Refactor scatter and image viewers

CHANGES.md

@@ -11,10 +11,10 @@ v0.11.0 (unreleased)
   references, which in turn caused some callback functions to not be
   cleaned up. [#1281]
 
-* Rewrote the histogram viewer to use the new state infrastructure. This
-  significantly simplifies the actual histogram viewer code both in terms
-  of number of lines and in terms of the number of connections/callbacks
-  that need to be set up manually. [#1278]
+* Rewrote the histogram, scatter, and image viewers to use the new state
+  infrastructure. This significantly simplifies the actual histogram viewer code
+  both in terms of number of lines and in terms of the number of
+  connections/callbacks that need to be set up manually. [#1278, #1289]
 
 v0.10.4 (2017-05-23)
 --------------------

appveyor.yml

@@ -10,7 +10,7 @@ environment:
       PYTHON_ARCH: "64" # needs to be set for CMD_IN_ENV to succeed. If a mix
                         # of 32 bit and 64 bit builds are needed, move this
                         # to the matrix section.
-      CONDA_DEPENDENCIES: "scipy cython pyqt matplotlib h5py pygments pyzmq scikit-image pandas xlrd pillow pytest mock coverage ipython ipykernel qtconsole traitlets qtpy"
+      CONDA_DEPENDENCIES: "astropy scipy cython pyqt matplotlib h5py pygments pyzmq scikit-image pandas xlrd pillow pytest mock coverage ipython ipykernel qtconsole traitlets qtpy"
       PIP_DEPENDENCIES: "plotly"
 
   matrix:

doc/conf.py

@@ -334,14 +334,8 @@
                   ('py:class', 'glue.viewers.scatter.layer_artist.ScatterLayerBase'),
                   ('py:class', 'glue.viewers.image.layer_artist.ImageLayerBase'),
                   ('py:class', 'glue.viewers.image.layer_artist.RGBImageLayerBase'),
-                  ('py:class', 'glue.viewers.image.qt.ImageWidgetBase'),
                   ('py:class', 'PyQt4.QtGui.QMainWindow'),
                   ('py:class', 'PyQt4.QtGui.QWidget'),
-                  # TODO: the next ones should be fixable
-                  ('py:obj', 'glue.viewers.image.qt.viewer_widget.ImageWidget.data'),
-                  ('py:obj', 'glue.viewers.image.qt.viewer_widget.ImageWidget.attribute'),
-                  ('py:obj', 'glue.viewers.image.qt.viewer_widget.ImageWidget.rgb_mode'),
-                  ('py:obj', 'glue.viewers.image.qt.viewer_widget.ImageWidget.slice'),
                   ('py:mod', 'glue.core'),
                   ('py:mod', 'glue.viewers'),
                   ('py:mod', 'glue.viewers.scatter'),

doc/customizing_guide/toolbar.rst

@@ -146,8 +146,8 @@ attribute on viewers:
 
 .. code:: python
 
-    >>> from glue.viewers.image.qt import ImageWidget
-    >>> ImageWidget.tools
+    >>> from glue.viewers.image.qt import ImageViewer
+    >>> ImageViewer.tools
     ['select:rectangle', 'select:xrange', 'select:yrange',
      'select:circle', 'select:polygon', 'image:colormap']
 
@@ -157,8 +157,8 @@ can therefore simply do e.g.:
 
 .. code:: python
 
-    from glue.viewers.image.qt import ImageWidget
-    ImageWidget.tools.append('custom_tool')
+    from glue.viewers.image.qt import ImageViewer
+    ImageViewer.tools.append('custom_tool')
 
 Including toolbars in custom viewers
 ------------------------------------
@@ -187,7 +187,7 @@ we defined above). There are currently two main classes available for toolbars:
 * :class:`~glue.viewers.common.qt.toolbar.BasicToolbar`: this is the most basic
   kind of toolbar - it comes with no tools by default.
 
-* :class:`~glue.viewers.common.qt.mpl_toolbar.MatplotlibViewerToolbar`: this is
+* :class:`~glue.viewers.matplotlib.qt.toolbar.MatplotlibViewerToolbar`: this is
   a subclass of :class:`~glue.viewers.common.qt.toolbar.BasicToolbar` that
   includes the standard Matplotlib buttons by default (home, zoom, pan, etc.).
   This toolbar can only be used if your data viewer includes a Matplotlib canvas

doc/developer_guide/api.rst

@@ -56,16 +56,24 @@ User Interface
 .. automodapi:: glue.viewers.common.qt.data_viewer
    :no-inheritance-diagram:
 
-.. automodapi:: glue.viewers.common.mpl_state
+.. automodapi:: glue.viewers.matplotlib.state
    :no-inheritance-diagram:
 
-.. automodapi:: glue.viewers.common.qt.mpl_data_viewer
+.. automodapi:: glue.viewers.matplotlib.qt.data_viewer
    :no-inheritance-diagram:
 
-.. automodapi:: glue.viewers.scatter.qt.viewer_widget
+.. automodapi:: glue.viewers.scatter.state
    :no-inheritance-diagram:
+   :inherited-members:
+
+.. automodapi:: glue.viewers.scatter.qt.data_viewer
+   :no-inheritance-diagram:
+
+.. automodapi:: glue.viewers.image.state
+   :no-inheritance-diagram:
+   :inherited-members:
 
-.. automodapi:: glue.viewers.image.qt.viewer_widget
+.. automodapi:: glue.viewers.image.qt.data_viewer
    :no-inheritance-diagram:
 
 .. automodapi:: glue.viewers.histogram.state
@@ -108,7 +116,7 @@ Viewers
    :no-inheritance-diagram:
    :inherited-members:
 
-.. automodapi:: glue.viewers.common.qt.mpl_toolbar
+.. automodapi:: glue.viewers.matplotlib.qt.toolbar
    :no-inheritance-diagram:
    :inherited-members:
 

doc/python_guide/data_viewer_options.rst

@@ -9,7 +9,7 @@ For example::
 
     from glue.core import Data, DataCollection
     from glue.app.qt.application import GlueApplication
-    from glue.viewers.scatter.qt import ScatterWidget
+    from glue.viewers.scatter.qt import ScatterViewer
     import numpy as np
 
     # create some data
@@ -20,7 +20,7 @@ For example::
     ga = GlueApplication(dc)
 
     # plot x vs y, flip the x axis, log-scale y axis
-    scatter = ga.new_data_viewer(ScatterWidget)
+    scatter = ga.new_data_viewer(ScatterViewer)
     scatter.add_data(d)
     scatter.xatt = d.id['x']
     scatter.yatt = d.id['y']
@@ -36,34 +36,25 @@ Plot Options
 
 Here are the settings associated with each data viewer:
 
-.. currentmodule:: glue.viewers.scatter.qt.viewer_widget
-
-:class:`Scatter Plots <ScatterWidget>`
---------------------------------------
-
-.. autosummary::
-    ~ScatterWidget.xlog
-    ~ScatterWidget.ylog
-    ~ScatterWidget.xflip
-    ~ScatterWidget.yflip
-    ~ScatterWidget.xmin
-    ~ScatterWidget.xmax
-    ~ScatterWidget.ymin
-    ~ScatterWidget.ymax
-    ~ScatterWidget.hidden
-    ~ScatterWidget.xatt
-    ~ScatterWidget.yatt
-
-.. currentmodule:: glue.viewers.image.qt.viewer_widget
-
-:class:`Image Viewer <ImageWidget>`
-------------------------------------
-
-.. autosummary::
-    ~ImageWidget.data
-    ~ImageWidget.attribute
-    ~ImageWidget.rgb_mode
-    ~ImageWidget.slice
+Scatter Viewer
+--------------
+
+The :class:`~glue.viewers.scatter.qt.data_viewer.ScatterViewer` class has a
+``state`` attribute which is an instance of
+:class:`~glue.viewers.scatter.state.ScatterViewerState`. To modify any settings
+in the viewer, set the appropriate attributes on ``state``, for example
+``state.x_min``. See :class:`~glue.viewers.scatter.state.ScatterViewerState`
+to find out the full list of available attributes.
+
+Image Viewer
+------------
+
+The :class:`~glue.viewers.image.qt.data_viewer.ImageViewer` class has a
+``state`` attribute which is an instance of
+:class:`~glue.viewers.image.state.ImageViewerState`. To modify any settings
+in the viewer, set the appropriate attributes on ``state``, for example
+``state.x_min``. See :class:`~glue.viewers.image.state.ImageViewerState`
+to find out the full list of available attributes.
 
 Histogram Viewer
 ----------------

glue/app/qt/application.py

@@ -23,10 +23,10 @@
 from glue.app.qt.mdi_area import GlueMdiArea, GlueMdiSubWindow
 from glue.app.qt.layer_tree_widget import PlotAction, LayerTreeWidget
 from glue.app.qt.preferences import PreferencesDialog
-from glue.viewers.common.qt.mpl_widget import defer_draw
+from glue.viewers.matplotlib.qt.widget import defer_draw
 from glue.viewers.common.qt.data_viewer import DataViewer
-from glue.viewers.image.qt import ImageWidget
-from glue.viewers.scatter.qt import ScatterWidget
+from glue.viewers.scatter.qt import ScatterViewer
+from glue.viewers.image.qt import ImageViewer
 from glue.utils import nonpartial
 from glue.utils.qt import (pick_class, GlueTabBar,
                            set_cursor_cm, messagebox_on_error, load_ui)
@@ -675,10 +675,10 @@ def choose_new_data_viewer(self, data=None):
 
         from glue.config import qt_client
 
-        if data and data.ndim == 1 and ScatterWidget in qt_client.members:
-            default = ScatterWidget
-        elif data and data.ndim > 1 and ImageWidget in qt_client.members:
-            default = ImageWidget
+        if data and data.ndim == 1 and ScatterViewer in qt_client.members:
+            default = ScatterViewer
+        elif data and data.ndim > 1 and ImageViewer in qt_client.members:
+            default = ImageViewer
         else:
             default = None
 

glue/app/qt/tests/test_application.py

@@ -20,8 +20,8 @@
 from glue.core.tests.test_state import Cloner, containers_equal, doubler, clone
 from glue.tests.helpers import requires_ipython
 from glue.utils.qt import process_dialog
-from glue.viewers.image.qt import ImageWidget
-from glue.viewers.scatter.qt import ScatterWidget
+from glue.viewers.image.qt import ImageViewer
+from glue.viewers.scatter.qt import ScatterViewer
 from glue.viewers.histogram.qt import HistogramViewer
 
 
@@ -136,20 +136,20 @@ def test_new_data_viewer_cancel(self):
     def test_new_data_viewer(self):
         with patch('glue.app.qt.application.pick_class') as pc:
 
-            pc.return_value = ScatterWidget
+            pc.return_value = ScatterViewer
 
             ct = len(self.app.current_tab.subWindowList())
 
             self.app.choose_new_data_viewer()
             assert len(self.app.current_tab.subWindowList()) == ct + 1
 
     def test_move(self):
-        viewer = self.app.new_data_viewer(ScatterWidget)
+        viewer = self.app.new_data_viewer(ScatterViewer)
         viewer.move(10, 20)
         assert viewer.position == (10, 20)
 
     def test_resize(self):
-        viewer = self.app.new_data_viewer(ScatterWidget)
+        viewer = self.app.new_data_viewer(ScatterViewer)
         viewer.viewer_size = (100, 200)
         assert viewer.viewer_size == (100, 200)
 
@@ -164,11 +164,11 @@ def test_new_data_defaults(self):
 
             self.app.choose_new_data_viewer(data=d1)
             args, kwargs = pc.call_args
-            assert kwargs['default'] is ScatterWidget
+            assert kwargs['default'] is ScatterViewer
 
             self.app.choose_new_data_viewer(data=d2)
             args, kwargs = pc.call_args
-            assert kwargs['default'] is ImageWidget
+            assert kwargs['default'] is ImageViewer
 
     def test_drop_load_data(self):
         m = QtCore.QMimeData()
@@ -297,7 +297,7 @@ def test_scatter_viewer(self):
         d = Data(label='x', x=[1, 2, 3, 4, 5], y=[2, 3, 4, 5, 6])
         dc = DataCollection([d])
         app = GlueApplication(dc)
-        w = app.new_data_viewer(ScatterWidget, data=d)
+        w = app.new_data_viewer(ScatterViewer, data=d)
         self.check_clone(app)
 
         s1 = dc.new_subset_group()

glue/app/qt/tests/test_preferences.py

@@ -10,8 +10,8 @@
 from qtpy import QtWidgets
 from glue.app.qt.preferences import PreferencesDialog
 from glue.app.qt import GlueApplication
-from glue.viewers.scatter.qt import ScatterWidget
-from glue.viewers.image.qt import ImageWidget
+from glue.viewers.scatter.qt import ScatterViewer
+from glue.viewers.image.qt import ImageViewer
 from glue.viewers.histogram.qt import HistogramViewer
 from glue.plugins.dendro_viewer.qt.viewer_widget import DendroWidget
 
@@ -307,10 +307,10 @@ def test_foreground_background_settings():
     # Make sure that settings change existing viewers, so we create a bunch of
     # viewers here.
 
-    scatter1 = app.new_data_viewer(ScatterWidget)
+    scatter1 = app.new_data_viewer(ScatterViewer)
     scatter1.add_data(d_1d)
 
-    image1 = app.new_data_viewer(ImageWidget)
+    image1 = app.new_data_viewer(ImageViewer)
     image1.add_data(d_2d)
 
     histogram1 = app.new_data_viewer(HistogramViewer)
@@ -354,10 +354,10 @@ def test_foreground_background_settings():
 
         # Now make sure that new viewers also inherit these settings
 
-        scatter2 = app.new_data_viewer(ScatterWidget)
+        scatter2 = app.new_data_viewer(ScatterViewer)
         scatter2.add_data(d_1d)
 
-        image2 = app.new_data_viewer(ImageWidget)
+        image2 = app.new_data_viewer(ImageViewer)
         image2.add_data(d_2d)
 
         histogram2 = app.new_data_viewer(HistogramViewer)

glue/core/component_id.py

@@ -8,7 +8,7 @@
 from glue.core.subset import InequalitySubsetState
 
 
-__all__ = ['PixelComponentID', 'ComponentID', 'PixelComponentID', 'ComponentIDDict', 'ComponentIDList']
+__all__ = ['ComponentID', 'PixelComponentID', 'ComponentIDDict', 'ComponentIDList']
 
 # access to ComponentIDs via .item[name]
 

glue/core/data.py

@@ -63,7 +63,7 @@ class Data(object):
     See also: :ref:`data_tutorial`
     """
 
-    def __init__(self, label="", **kwargs):
+    def __init__(self, label="", coords=None, **kwargs):
         """
 
         :param label: label for data
@@ -72,7 +72,7 @@ def __init__(self, label="", **kwargs):
         Extra array-like keywords are extracted into components
         """
         # Coordinate conversion object
-        self.coords = Coordinates()
+        self.coords = coords or Coordinates()
         self._shape = ()
 
         # Components

glue/core/layer_artist.py

@@ -294,15 +294,8 @@ def _duplicate(self, artist):
                 return True
         return False
 
-    def _check_duplicate(self, artist):
-        """Raise an error if this artist is a duplicate"""
-        if self._duplicate(artist):
-            raise ValueError("Already have an artist for this type "
-                             "and data")
-
     def append(self, artist):
         """Add a LayerArtist to this collection"""
-        self._check_duplicate(artist)
         self.artists.append(artist)
         artist.zorder = max(a.zorder for a in self.artists) + 1
         self._notify()