Repository: csiro-coasts/emsarray

Issue:
Error when plotting variables with no 'long_name' attribute
Plotting a data array using `Convention.plot(data_array)` when the data array has no 'long_name' attribute but does have a single time coordinate fails. `Convention.plot_on_figure()` attempts to generate a title from the long_name and time coordinate. This fails as it tries to string concatenate the 'long_name' (None) with the formatted time value (a string).

The automatic title generation should handle this case by only using the time coordinate, and not assuming that the long_name will always have a string value.

Discussion:

Patch:
diff --git a/docs/releases/development.rst b/docs/releases/development.rst
index d1ec009..24fb7a8 100644
--- a/docs/releases/development.rst
+++ b/docs/releases/development.rst
@@ -31,3 +31,5 @@ Next release (in development)
   This feature is considered experimental
   and may change significantly in future releases.
   (:pr:`110`).
+* Fix an issue with plotting variables with no ``long_name`` attribute.
+  (:issue:`105`, :pr:`111`).
diff --git a/src/emsarray/conventions/_base.py b/src/emsarray/conventions/_base.py
index 811cd82..fcfeaea 100644
--- a/src/emsarray/conventions/_base.py
+++ b/src/emsarray/conventions/_base.py
@@ -938,7 +938,10 @@ class Convention(abc.ABC, Generic[GridKind, Index]):
             #
             # Users can supply their own titles
             # if this automatic behaviour is insufficient
-            kwargs['title'] = kwargs['scalar'].attrs.get('long_name')
+            title_bits: list[str] = []
+            long_name = kwargs['scalar'].attrs.get('long_name')
+            if long_name is not None:
+                title_bits.append(str(long_name))
             try:
                 time_coordinate = self.dataset.variables[self.get_time_name()]
             except KeyError:
@@ -949,8 +952,11 @@ class Convention(abc.ABC, Generic[GridKind, Index]):
                 # as long as the time coordinate is a proper coordinate with
                 # matching dimension name, not an auxiliary coordinate.
                 if time_coordinate.size == 1:
-                    time = time_coordinate.values
-                    kwargs['title'] = kwargs['title'] + '\n' + str(time)
+                    time = time_coordinate.values[0]
+                    title_bits.append(str(time))
+
+            if title_bits:
+                kwargs['title'] = '\n'.join(title_bits)
 
         plot_on_figure(figure, self, **kwargs)
 


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index 2a03760..2896f55 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1,3 +1,4 @@
+import logging
 import pathlib
 from unittest import mock
 
@@ -5,6 +6,8 @@ import pytest
 
 import emsarray
 
+logger = logging.getLogger(__name__)
+
 
 @pytest.fixture
 def datasets() -> pathlib.Path:
@@ -19,6 +22,7 @@ def pytest_runtest_setup(item):
 
 @pytest.fixture
 def matplotlib_backend(
+    request: pytest.FixtureRequest,
     monkeypatch: pytest.MonkeyPatch,
     backend: str = 'template',
 ):
@@ -28,6 +32,12 @@ def matplotlib_backend(
     import matplotlib
     import matplotlib.pyplot
 
+    node = request.node
+    mark = node.get_closest_marker('matplotlib')
+
+    if mark.kwargs.get('mock_coast', False):
+        monkeypatch.setattr(emsarray.plot, 'add_coast', lambda figure: None)
+
     show_mock = mock.Mock(spec=matplotlib.pyplot.show)
     monkeypatch.setattr(matplotlib.pyplot, 'show', show_mock)
 
diff --git a/tests/test_plot.py b/tests/test_plot.py
index 9f5784f..2942b03 100644
--- a/tests/test_plot.py
+++ b/tests/test_plot.py
@@ -1,11 +1,18 @@
+import logging
+import pathlib
+
 import cartopy.crs
 import matplotlib.figure
+import matplotlib.pyplot
 import numpy
 import pytest
 import shapely
 
+import emsarray
 from emsarray.plot import add_landmarks, polygons_to_collection
 
+logger = logging.getLogger(__name__)
+
 
 @pytest.mark.matplotlib
 def test_polygons_to_collection():
@@ -43,3 +50,52 @@ def test_add_landmarks():
     for landmark, text in zip(landmarks, axes.texts):
         assert text.get_text() == landmark[0]
         assert text.xy == landmark[1].coords.xy
+
+
+@pytest.mark.matplotlib(mock_coast=True)
+@pytest.mark.tutorial
+def test_plot(
+    datasets: pathlib.Path,
+    tmp_path: pathlib.Path,
+):
+    """
+    Test plotting a variable with no long_name attribute works.
+    Regression test for https://github.com/csiro-coasts/emsarray/issues/105
+    """
+    dataset = emsarray.tutorial.open_dataset('gbr4')
+    temp = dataset['temp'].copy()
+    temp = temp.isel(time=0, k=-1)
+
+    dataset.ems.plot(temp)
+
+    figure = matplotlib.pyplot.gcf()
+    axes = figure.axes[0]
+    assert axes.get_title() == 'Temperature\n2022-05-11T14:00:00.000000000'
+
+    matplotlib.pyplot.savefig(tmp_path / 'plot.png')
+    logger.info("Saved plot to %r", tmp_path / 'plot.png')
+
+
+@pytest.mark.matplotlib(mock_coast=True)
+@pytest.mark.tutorial
+def test_plot_no_long_name(
+    datasets: pathlib.Path,
+    tmp_path: pathlib.Path,
+):
+    """
+    Test plotting a variable with no long_name attribute works.
+    Regression test for https://github.com/csiro-coasts/emsarray/issues/105
+    """
+    dataset = emsarray.tutorial.open_dataset('gbr4')
+    temp = dataset['temp'].copy()
+    temp = temp.isel(time=0, k=-1)
+    del temp.attrs['long_name']
+
+    dataset.ems.plot(temp)
+
+    figure = matplotlib.pyplot.gcf()
+    axes = figure.axes[0]
+    assert axes.get_title() == '2022-05-11T14:00:00.000000000'
+
+    matplotlib.pyplot.savefig(tmp_path / 'plot.png')
+    logger.info("Saved plot to %r", tmp_path / 'plot.png')