Merge pull request #2293 from nabobalis/preferred_cmap

Raise error when passing in an incorrect string or object for a preferred colormap for VisualAttributes

.gitignore

@@ -42,3 +42,5 @@ glue/qt/glue_qt_resources.py
 .pytest_cache
 .tox
 .vscode
+# vscode plugin
+.history

glue/core/tests/test_visual.py

@@ -0,0 +1,30 @@
+from glue.core.visual import VisualAttributes
+from matplotlib.cm import get_cmap
+import pytest
+
+
+def test_VA_preferred_cmap():
+    # Not a real CMAP array - errors
+    with pytest.raises(TypeError, match="`preferred_cmap` must be a string or an instance of a matplotlib.colors.Colormap"):
+        VisualAttributes(preferred_cmap=1)
+
+    # Not a valid string for a CMAP - errors
+    with pytest.raises(ValueError, match="not_a_cmap is not a valid colormap name."):
+        VisualAttributes(preferred_cmap="not_a_cmap")
+
+    viridis_cmap = get_cmap("viridis")
+
+    # get_cmap cmap name
+    va = VisualAttributes(preferred_cmap="viridis")
+    assert va.preferred_cmap == viridis_cmap
+    # formal cmap name
+    va = VisualAttributes(preferred_cmap="Viridis")
+    assert va.preferred_cmap == viridis_cmap
+
+    # Valid Colormap
+    va = VisualAttributes(preferred_cmap=viridis_cmap)
+    assert va.preferred_cmap == viridis_cmap
+
+    # None is allowed - it is the default
+    va = VisualAttributes(preferred_cmap=None)
+    assert va.preferred_cmap is None

glue/core/visual.py

@@ -1,4 +1,4 @@
-from matplotlib.colors import ColorConverter
+from matplotlib.colors import ColorConverter, Colormap
 from matplotlib.cm import get_cmap
 
 from glue.config import settings
@@ -15,20 +15,29 @@
 class VisualAttributes(HasCallbackProperties):
 
     """
-    This class is used to define visual attributes for any kind of objects
-
-    The essential attributes of a VisualAttributes instance are:
-
-    :param color: A matplotlib color string
-    :param alpha: Opacity (0-1)
-    :param linewidth: The linewidth (float or int)
-    :param linestyle: The linestyle (``'solid' | 'dashed' | 'dash-dot' | 'dotted' | 'none'``)
-    :param marker: The matplotlib marker shape (``'o' | 's' | '^' | etc``)
-    :param markersize: The size of the marker (int)
-
+    This class is used to define visual attributes for any kind of objects.
+
+    Parameters
+    ----------
+    parent : `QObject`, optional
+        The object that this visual attributes object is attached to. Default is `None`.
+    color : `str`, optional
+        A matplotlib color string. Default is set from :class:`~glue.config.SettingRegistry`.
+    alpha : `float`, optional
+        Opacity, between 0-1. Default is set from :class:`~glue.config.SettingRegistry`.
+    preferred_cmap : `str` or :class:`~matplotlib.colors.Colormap`, optional
+        A colormap to be used as the preferred colormap, by name or instance. Default is `None`.
+    linewidth : `float`, optional
+        The linewidth. Default is 1.
+    linestyle : `str`, optional
+        The linestyle. Default is `'solid'`.
+    marker : `str`, optional
+        The matplotlib marker shape. Default is `'o'`.
+    markersize : `float`, optional
+        The size of the marker. Default is 3.
     """
 
-    def __init__(self, parent=None, washout=False, color=None, alpha=None, preferred_cmap=None):
+    def __init__(self, parent=None, color=None, alpha=None, preferred_cmap=None, linewidth=1, linestyle='solid', marker='o', markersize=3):
 
         super(VisualAttributes, self).__init__()
 
@@ -43,10 +52,10 @@ def __init__(self, parent=None, washout=False, color=None, alpha=None, preferred
         self.color = color
         self.alpha = alpha
         self.preferred_cmap = preferred_cmap
-        self.linewidth = 1
-        self.linestyle = 'solid'
-        self.marker = 'o'
-        self.markersize = 3
+        self.linewidth = linewidth
+        self.linestyle = linestyle
+        self.marker = marker
+        self.markersize = markersize
 
     def __eq__(self, other):
         if not isinstance(other, VisualAttributes):
@@ -109,13 +118,21 @@ def preferred_cmap(self):
     def preferred_cmap(self, value):
         if isinstance(value, str):
             try:
-                for i, element in enumerate(colormaps.members):
+                self._preferred_cmap = get_cmap(value)
+            except ValueError:
+                # This checks for the formal name of the colormap.
+                # e.g., 'viridis' is 'Viridis'
+                for element in colormaps.members:
                     if element[0] == value:
                         self._preferred_cmap = element[1]
-            except TypeError:
-                self._preferred_cmap = get_cmap(value)
-        else:
+                        break
+                else:
+                    # If the string name fails to be validated
+                    raise ValueError(f"{value} is not a valid colormap name.")
+        elif isinstance(value, Colormap) or value is None:
             self._preferred_cmap = value
+        else:
+            raise TypeError("`preferred_cmap` must be a string or an instance of a matplotlib.colors.Colormap")
 
     @callback_property
     def alpha(self):