Correct TiledDataset.plot mosaic ordering (#504)

* Axis grid produced by `TiledDataset.plot` now has origin at lower left

Previously it had been the matplotlib default of the upper left, which didn't
look right.

* Add option to have correctly-ordered limits in tiled dataset plot

* Add changelog fragments

* Add test coverage that non-square TiledDatasets are plotted correctly

And update image hashes

* Add test coverage for `limits_from_wcs` option

Needed to flip sample dataset WCS to have the option actually do anything

* Split test of non-square and swapped `TiledDataset.plot` into new test

* Whitespace adjustments

* Use `ax.wcs` instead of passing a different WCS object

They should be the same

* Replace automatic WCS flipping with a simple switch for users

The automatic logic was becoming increasingly fragile and complicated. Much
easier to let the user decide what they want to do.

* Add `DKISTUserWarning` if calling `TiledDataset.plot` on stale metadata file

* Catch warnings caused by currently stale sample data

* Update changelog fragment talking about new `.plot` kwarg

changelog/504.feature.2.rst

@@ -0,0 +1,2 @@
+Add `swap_tile_limits` kwarg to `TiledDataset.plot`.
+This option allows the user to invert plot limits on either axes to account for WCS values that decrease compared to the pixel axes.

changelog/504.feature.rst

@@ -0,0 +1,3 @@
+Update grid orientation of `TiledDataset.plot`.
+The grid now has MAXIS1 columns and MAXIS2 rows where MINDEX1 corresponds to column and MINDEX2 corresponds to row.
+Additionally, the origin for the grid is now in the lower-left as opposed to the upper-left.

dkist/dataset/tests/test_tiled_dataset.py

@@ -6,6 +6,7 @@
 
 from dkist import Dataset, TiledDataset, load_dataset
 from dkist.tests.helpers import figure_test
+from dkist.utils.exceptions import DKISTUserWarning
 
 
 def test_tiled_dataset(simple_tiled_dataset, dataset):
@@ -79,6 +80,37 @@ def test_tiled_dataset_from_components(dataset):
 def test_tileddataset_plot(share_zscale):
     from dkist.data.sample import VBI_AJQWW
     ori_ds = load_dataset(VBI_AJQWW)
+
+    newtiles = []
+    for tile in ori_ds.flat:
+        newtiles.append(tile.rebin((1, 8, 8), operation=np.sum))
+    # ndcube 2.3.0 introduced a deepcopy for rebin, this broke our dataset validation
+    # https://github.com/sunpy/ndcube/issues/815
+    for tile in newtiles:
+        tile.meta["inventory"] = ori_ds.inventory
+    ds = TiledDataset(np.array(newtiles).reshape(ori_ds.shape), meta={"inventory": newtiles[0].inventory})
+
+    fig = plt.figure(figsize=(12, 15))
+    with pytest.warns(DKISTUserWarning,
+                      match="The metadata ASDF file that produced this dataset is out of date and will result in "
+                            "incorrect plots. Please re-download the metadata ASDF file."):
+        #TODO: Once sample data have been updated maybe we should test both paths here (old data and new data)
+        ds.plot(0, share_zscale=share_zscale, figure=fig)
+
+    return plt.gcf()
+
+@figure_test
+@pytest.mark.parametrize("swap_tile_limits", ["x", "y", "xy", None])
+def test_tileddataset_plot_limit_swapping(swap_tile_limits):
+    # Also test that row/column sizes are correct
+
+    from dkist.data.sample import VBI_AJQWW
+    ori_ds = load_dataset(VBI_AJQWW)
+
+    # Swap WCS to make the `swap_tile_limits` option more natural
+    for tile in ori_ds.flat:
+        tile.wcs.forward_transform[0].cdelt *= -1
+
     newtiles = []
     for tile in ori_ds.flat:
         newtiles.append(tile.rebin((1, 8, 8), operation=np.sum))
@@ -87,8 +119,30 @@ def test_tileddataset_plot(share_zscale):
     for tile in newtiles:
         tile.meta["inventory"] = ori_ds.inventory
     ds = TiledDataset(np.array(newtiles).reshape(ori_ds.shape), meta={"inventory": newtiles[0].inventory})
+
+    non_square_ds = ds[:2, :]
+    assert non_square_ds.shape[0] != non_square_ds.shape[1]  # Just in case the underlying data change for some reason
+
     fig = plt.figure(figsize=(12, 15))
-    ds.plot(0, share_zscale=share_zscale, figure=fig)
+    with pytest.warns(DKISTUserWarning,
+                      match="The metadata ASDF file that produced this dataset is out of date and will result in "
+                            "incorrect plots. Please re-download the metadata ASDF file."):
+        #TODO: Once sample data have been updated maybe we should test both paths here (old data and new data)
+        non_square_ds.plot(0, share_zscale=False, swap_tile_limits=swap_tile_limits, figure=fig)
+
+    assert fig.axes[0].get_gridspec().get_geometry() == non_square_ds.shape[::-1]
+    for ax in fig.axes:
+        xlims = ax.get_xlim()
+        ylims = ax.get_ylim()
+
+        if swap_tile_limits in ["x", "xy"]:
+            assert xlims[0] > xlims[1]
+        if swap_tile_limits in ["y", "xy"]:
+            assert ylims[0] > ylims[1]
+        if swap_tile_limits is None:
+            assert xlims[0] < xlims[1]
+            assert ylims[0] < ylims[1]
+
     return plt.gcf()
 
 

dkist/dataset/tiled_dataset.py

@@ -6,18 +6,20 @@
 not contiguous in the spatial dimensions (due to overlaps and offsets).
 """
 import warnings
+from typing import Literal
 from textwrap import dedent
 from collections.abc import Collection
 
 import matplotlib.pyplot as plt
 import numpy as np
+from matplotlib.gridspec import GridSpec
 
 import astropy
 from astropy.table import vstack
 
 from dkist.io.file_manager import FileManager, StripedExternalArray
 from dkist.io.loaders import AstropyFITSLoader
-from dkist.utils.exceptions import DKISTDeprecationWarning
+from dkist.utils.exceptions import DKISTDeprecationWarning, DKISTUserWarning
 
 from .dataset import Dataset
 from .utils import dataset_info_str
@@ -184,7 +186,7 @@ def _get_axislabels(ax):
                 ylabel = coord.get_axislabel() or coord._get_default_axislabel()
         return (xlabel, ylabel)
 
-    def plot(self, slice_index, share_zscale=False, figure=None, **kwargs):
+    def plot(self, slice_index, share_zscale=False, figure=None, swap_tile_limits: Literal["x", "y", "xy"] | None = None, **kwargs):
         """
         Plot a slice of each tile in the TiledDataset
 
@@ -201,32 +203,63 @@ def plot(self, slice_index, share_zscale=False, figure=None, **kwargs):
         figure : `matplotlib.figure.Figure`
             A figure to use for the plot. If not specified the current pyplot
             figure will be used, or a new one created.
+        swap_tile_limits : `"x", "y", "xy"` or `None` (default)
+            Invert the axis limits of each tile. Either the "x" or "y" axis limits can be inverted separately, or they
+            can both be inverted with "xy". This option is useful if the orientation of the tile data arrays is flipped
+            w.r.t. the WCS orientation implied by the mosaic keys. For example, most DL-NIRSP data should be plotted with
+            `swap_tile_limits="xy"`.
         """
+        if swap_tile_limits not in ["x", "y", "xy", None]:
+            raise RuntimeError("swap_tile_limits must be one of ['x', 'y', 'xy', None]")
+
+        if len(self.meta.get("history", {}).get("entries", [])) == 0:
+            warnings.warn("The metadata ASDF file that produced this dataset is out of date and "
+                          "will result in incorrect plots. Please re-download the metadata ASDF file.",
+                          DKISTUserWarning)
+
         if isinstance(slice_index, int):
             slice_index = (slice_index,)
         vmin, vmax = np.inf, 0
 
         if figure is None:
             figure = plt.gcf()
 
-        tiles = self.slice_tiles[slice_index].flat
-        for i, tile in enumerate(tiles):
-            ax = figure.add_subplot(self.shape[0], self.shape[1], i+1, projection=tile.wcs)
-            tile.plot(axes=ax, **kwargs)
-            if i == 0:
-                xlabel, ylabel = self._get_axislabels(ax)
-                figure.supxlabel(xlabel, y=0.05)
-                figure.supylabel(ylabel, x=0.05)
-            axmin, axmax = ax.get_images()[0].get_clim()
-            vmin = axmin if axmin < vmin else vmin
-            vmax = axmax if axmax > vmax else vmax
-            ax.set_ylabel(" ")
-            ax.set_xlabel(" ")
+        sliced_dataset = self.slice_tiles[slice_index]
+        dataset_ncols, dataset_nrows = sliced_dataset.shape
+        gridspec = GridSpec(nrows=dataset_nrows, ncols=dataset_ncols, figure=figure)
+        for col in range(dataset_ncols):
+            for row in range(dataset_nrows):
+                tile = sliced_dataset[col, row]
+
+                # Fill up grid from the bottom row
+                ax_gridspec = gridspec[dataset_nrows - row - 1, col]
+                ax = figure.add_subplot(ax_gridspec, projection=tile.wcs)
+
+                tile.plot(axes=ax, **kwargs)
+
+                if swap_tile_limits in ["x", "xy"]:
+                    ax.invert_xaxis()
+
+                if swap_tile_limits in ["y", "xy"]:
+                    ax.invert_yaxis()
+
+                ax.set_ylabel(" ")
+                ax.set_xlabel(" ")
+                if col == row == 0:
+                    xlabel, ylabel = self._get_axislabels(ax)
+                    figure.supxlabel(xlabel, y=0.05)
+                    figure.supylabel(ylabel, x=0.05)
+
+                axmin, axmax = ax.get_images()[0].get_clim()
+                vmin = axmin if axmin < vmin else vmin
+                vmax = axmax if axmax > vmax else vmax
+
         if share_zscale:
             for ax in figure.get_axes():
                 ax.get_images()[0].set_clim(vmin, vmax)
+
         title = f"{self.inventory['instrumentName']} Dataset ({self.inventory['datasetId']}) at "
-        for i, (coord, val) in enumerate(list(tiles[0].global_coords.items())[::-1]):
+        for i, (coord, val) in enumerate(list(sliced_dataset.flat[0].global_coords.items())[::-1]):
             if coord == "time":
                 val = val.iso
             if coord == "stokes":

dkist/tests/figure_hashes_mpl_391_ft_261_astropy_611_animators_111_ndcube_222.json

@@ -6,6 +6,10 @@
   "dkist.dataset.tests.test_plotting.test_2d_plot[aslice1]": "cbb84fbae51d8238803f8f0d6820c575f024fe54b1656f1b181dc4ec645e9ff9",
   "dkist.dataset.tests.test_plotting.test_2d_plot[aslice2]": "132c5615832daff457dacb4cb770498f1fbb4460a5b90b5d4d01d224c70eeb28",
   "dkist.dataset.tests.test_plotting.test_2d_plot2": "409b5a10ad8ccf005331261505e63ce8febdc38eb8b5a34f8863e567e3cccb9c",
-  "dkist.dataset.tests.test_tiled_dataset.test_tileddataset_plot[share_zscale]": "859641d0884ef13a6575ca7125cecea0faaf3722702c22b9a59a728d6c7abe0e",
-  "dkist.dataset.tests.test_tiled_dataset.test_tileddataset_plot[indpendent_zscale]": "042305abd97f9a59522c5b5e5d7f5389fe010c604b08b9afe00ec7e5a49b7b65"
+  "dkist.dataset.tests.test_tiled_dataset.test_tileddataset_plot[share_zscale]": "40298abbc680c82de029b02c4e543a60eac1b2d71e06b22c53a1d43194491ac3",
+  "dkist.dataset.tests.test_tiled_dataset.test_tileddataset_plot[indpendent_zscale]": "b6f2dd9fdeb79bf25ad43a591d8dec242f32e0ba3a521e15791058d51e0ecbaf",
+  "dkist.dataset.tests.test_tiled_dataset.test_tileddataset_plot_limit_swapping[x]": "0f2fa941c020f9853eff0eaf2f575be193372d7042731349d166a4b3645d78b0",
+  "dkist.dataset.tests.test_tiled_dataset.test_tileddataset_plot_limit_swapping[y]": "ae3a81c58bf55afed01c90cac9ce6227cddf430c0741d9c2f7b2d4c3ca350a6f",
+  "dkist.dataset.tests.test_tiled_dataset.test_tileddataset_plot_limit_swapping[xy]": "9098876ebd47e11e2aca7460c29ac1614e383a2386868995ca3b57c61ace0162",
+  "dkist.dataset.tests.test_tiled_dataset.test_tileddataset_plot_limit_swapping[None]": "0159e3fcd0f7109e216888ea337e8eb9861dbc951ab9cfba5d14cc6c8b501132"
 }

dkist/tests/figure_hashes_mpl_dev_ft_261_astropy_dev_animators_dev_ndcube_dev.json

@@ -6,6 +6,10 @@
   "dkist.dataset.tests.test_plotting.test_2d_plot[aslice1]": "cbb84fbae51d8238803f8f0d6820c575f024fe54b1656f1b181dc4ec645e9ff9",
   "dkist.dataset.tests.test_plotting.test_2d_plot[aslice2]": "4b5be9cf1883d0ebd15ff091f52cea2822068e8238a8df7b0f594d69fba27597",
   "dkist.dataset.tests.test_plotting.test_2d_plot2": "1c10e9db44b0b694a6bb1b493c4c2193278541df7c1302bb11fe3f6372682e35",
-  "dkist.dataset.tests.test_tiled_dataset.test_tileddataset_plot[share_zscale]": "a5c5e439af14d99110858b552649a334ca2157f146c702cb5ce790fe6ba8ca1a",
-  "dkist.dataset.tests.test_tiled_dataset.test_tileddataset_plot[indpendent_zscale]": "71022ae3a7bbc2e1250cb5913479d72ad73570eb2e10dd463faf83a1e47865e7"
+  "dkist.dataset.tests.test_tiled_dataset.test_tileddataset_plot[share_zscale]": "bd0cfadd99f9d3d416f011184f2e9a7971df226879c8786e8ab2349e13909b5c",
+  "dkist.dataset.tests.test_tiled_dataset.test_tileddataset_plot[indpendent_zscale]": "2d6afac3f582846f4be95b23b524bb670895b0885519d8c13623307d07a3b39e",
+  "dkist.dataset.tests.test_tiled_dataset.test_tileddataset_plot_limit_swapping[x]": "b35593deb273b02ff1f2384810c4cf825ef5017ecad4d020543c53ad6361cd9e",
+  "dkist.dataset.tests.test_tiled_dataset.test_tileddataset_plot_limit_swapping[y]": "78de0395df62edd8626014d7b8924b5f3d1d66b27be9c1a328fac7b7639e702b",
+  "dkist.dataset.tests.test_tiled_dataset.test_tileddataset_plot_limit_swapping[xy]": "05219c0c450825fa7bd555ff27a9a111066082b15e2dde83ac2f3ac43dba5102",
+  "dkist.dataset.tests.test_tiled_dataset.test_tileddataset_plot_limit_swapping[None]": "64a247b0b54b7de8a8a7c636d60bdceb7d7581a5429c9e8d813b8d81912a2c10"
 }