Merge pull request #124 from Carifio24/dot-size-update

Update dotplot size calculation

Author: Carifio24

glue_plotly/common/dotplot.py

@@ -5,22 +5,27 @@
 
 from glue.core import BaseData
 
-from .common import color_info, dimensions
+from glue_plotly.common import color_info, dimensions
+from glue_plotly.viewers.common import PlotlyBaseView
 
 
-def dot_radius(viewer, layer_state):
+def dot_size(viewer, layer_state):
     edges = layer_state.histogram[0]
     viewer_state = viewer.state
     diam_world = min([edges[i + 1] - edges[i] for i in range(len(edges) - 1)])
     width, height = dimensions(viewer)
+    if isinstance(viewer, PlotlyBaseView):
+        margins = viewer.figure.layout.margin
+        if margins:
+            height -= (margins.b + margins.t)
+            width -= (margins.l + margins.r)
     diam = diam_world * width / abs(viewer_state.x_max - viewer_state.x_min)
     if viewer_state.y_min is not None and viewer_state.y_max is not None and viewer_state.y_min != viewer_state.y_max:
-        max_diam_world_v = 1
-        diam_pixel_v = max_diam_world_v * height / abs(viewer_state.y_max - viewer_state.y_min)
+        diam_pixel_v = height / abs(viewer_state.y_max - viewer_state.y_min)
         diam = min(diam_pixel_v, diam)
     if not isfinite(diam):
         diam = 1
-    return diam / 2
+    return diam * 0.95
 
 
 def dot_positions(layer_state):
@@ -43,7 +48,7 @@ def dots_for_layer(viewer, layer_state, add_data_label=True):
 
     x, y = dot_positions(layer_state)
 
-    radius = dot_radius(viewer, layer_state)
+    radius = dot_size(viewer, layer_state)
     marker = dict(color=color_info(layer_state, mask=None), size=radius)
 
     name = layer_state.layer.label

glue_plotly/common/tests/test_dotplot.py

@@ -1,3 +1,4 @@
+from glue_jupyter import JupyterApplication
 from numpy import unique
 from plotly.graph_objs import Scatter
 
@@ -6,7 +7,7 @@
 from glue_qt.viewers.histogram import HistogramViewer
 
 from glue_plotly.common import sanitize
-from glue_plotly.common.dotplot import dot_radius, traces_for_layer
+from glue_plotly.common.dotplot import dot_size, traces_for_layer
 
 from glue_plotly.viewers.histogram.viewer import PlotlyHistogramView
 from glue_plotly.viewers.histogram.dotplot_layer_artist import PlotlyDotplotLayerArtist
@@ -16,9 +17,89 @@ class SimpleDotplotViewer(PlotlyHistogramView):
     _data_artist_cls = PlotlyDotplotLayerArtist
     _subset_artist_cls = PlotlyDotplotLayerArtist
 
+    def close(self, warn=False):
+        self.figure.close()
+
 
 class TestDotplot:
 
+    def setup_method(self, method):
+        x = [86,  86,  76,  78,  93, 100,  90,  87,  73,  61,  71,  68,  78,
+             9,  87,  32,  34,   2,  57,  79,  48,   5,   8,  19,   7,  78,
+             16,  15,  58,  34,  20,  63,  96,  97,  86,  92,  35,  59,  75,
+             0,  53,  45,  59,  74,  59,   4,  69,  76,  97,  77,  24,  99,
+             50,   6,   1,  55,  13,  40,  27,  17,  92,  72,  40,  29,  64,
+             38,  77,  11,  91,  23,  59,  92,   5,  88,  15,  90,  40, 100,
+             47,  28,   3,  44,  89,  75,  13,  94,  95,  43,  17,  88,   6,
+             94, 100,  28,  45,  36,  63,  14,  90,  66]
+        self.data = Data(label="dotplot", x=x)
+        self.app = JupyterApplication()
+        self.app.session.data_collection.append(self.data)
+        self.viewer = self.app.new_data_viewer(SimpleDotplotViewer)
+        self.viewer.add_data(self.data)
+        self.mask, self.sanitized = sanitize(self.data['x'])
+
+        viewer_state = self.viewer.state
+        viewer_state.hist_n_bin = 18
+        viewer_state.x_axislabel_size = 14
+        viewer_state.y_axislabel_size = 8
+        viewer_state.x_ticklabel_size = 18
+        viewer_state.y_ticklabel_size = 20
+        viewer_state.x_min = 0
+        viewer_state.x_max = 100
+        viewer_state.y_min = 0
+        viewer_state.y_max = 15
+        viewer_state.x_axislabel = 'X Axis'
+        viewer_state.y_axislabel = 'Y Axis'
+
+        self.layer = self.viewer.layers[0]
+        self.layer.state.color = '#0e1dab'
+        self.layer.state.alpha = 0.85
+
+    def teardown_method(self, method):
+        self.viewer.close(warn=False)
+        self.viewer = None
+        self.app = None
+
+    def test_basic_dots(self):
+        traces = traces_for_layer(self.viewer, self.layer.state)
+        assert len(traces) == 1
+        dots = traces[0]
+        assert isinstance(dots, Scatter)
+
+        assert len(unique(dots.x)) == 18
+        expected_y = (1, 2, 3, 4, 5, 6, 7, 1, 2, 3, 4, 5, 6, 1,
+                      2, 3, 4, 5, 6, 1, 2, 3, 4, 1, 2, 3, 1, 2,
+                      3, 4, 1, 2, 3, 4, 5, 1, 2, 3, 4, 5, 1, 2,
+                      3, 4, 5, 1, 2, 1, 2, 3, 4, 5, 6, 7, 1, 2,
+                      3, 4, 1, 2, 3, 4, 1, 2, 3, 4, 5, 6, 7, 8,
+                      1, 2, 3, 4, 1, 2, 3, 4, 5, 6, 7, 1, 2, 3,
+                      4, 5, 6, 7, 8, 9, 10, 11, 1, 2, 3, 4, 5,
+                      6, 7, 8)
+
+        assert dots.y == expected_y
+
+        # Default figure is 900x600, with margins of 50 on each side
+        width = 900 - 50 - 50
+        height = 600 - 50 - 50
+        diam = 0.95 * min(height / 15, width / 18)
+        assert dots.marker.size == diam
+
+    def test_dot_radius_defined(self):
+        """
+        This test makes sure that we correctly get the default value for the dot radius
+        when both axes have no range.
+        """
+        self.viewer.state.x_min = 1
+        self.viewer.state.x_max = 1
+        self.viewer.state.y_min = 1
+        self.viewer.state.y_max = 1
+
+        assert dot_size(self.viewer, self.layer.state) == 0.95
+
+
+class TestDotsHistogram:
+
     def setup_method(self, method):
         x = [86,  86,  76,  78,  93, 100,  90,  87,  73,  61,  71,  68,  78,
              9,  87,  32,  34,   2,  57,  79,  48,   5,   8,  19,   7,  78,
@@ -75,7 +156,12 @@ def test_basic_dots(self):
                       6, 7, 8)
 
         assert dots.y == expected_y
-        assert dots.marker.size == 16  # Default figure is 640x480
+
+        # Default figure is 640x480
+        width = 640
+        height = 480
+        diam = 0.95 * min(height / 15, width / 18)
+        assert dots.marker.size == diam
 
     def test_dot_radius_defined(self):
         """
@@ -87,4 +173,4 @@ def test_dot_radius_defined(self):
         self.viewer.state.y_min = 1
         self.viewer.state.y_max = 1
 
-        assert dot_radius(self.viewer, self.layer.state) == 0.5
+        assert dot_size(self.viewer, self.layer.state) == 0.95

glue_plotly/viewers/histogram/dotplot_layer_artist.py

@@ -9,7 +9,7 @@
 from glue.viewers.histogram.state import HistogramLayerState
 from glue_plotly.common.common import fixed_color
 
-from glue_plotly.common.dotplot import dot_positions, dot_radius, dots_for_layer
+from glue_plotly.common.dotplot import dot_positions, dot_size, dots_for_layer
 
 __all__ = ["PlotlyDotplotLayerArtist"]
 
@@ -123,7 +123,7 @@ def _update_visual_attributes(self, changed, force=False):
 
     def _update_visual_attrs_for_trace(self, trace):
         marker = trace.marker
-        marker.update(opacity=self.state.alpha, color=fixed_color(self.state), size=dot_radius(self.view, self.state))
+        marker.update(opacity=self.state.alpha, color=fixed_color(self.state), size=dot_size(self.view, self.state))
         trace.update(marker=marker,
                      visible=self.state.visible,
                      unselected=dict(marker=dict(opacity=self.state.alpha)))