Repository: scipp/plopp

Issue:
figure3d (point cloud) needs flattened data but does not raise if it isn't
The `scatter3d` function flattens the data if it has more than one dimension, but the lower level `figure3d` does not.
If used directly, this can lead to unpredictable/error-inducing results, because it is not clear what `pythreejs` does with multi-dimensional data.

For example, making a flat-panel 2d detector:
```Py
import scipp as sc
import numpy as np
import plopp as pp

N = 50

x = sc.broadcast(sc.arange('x', float(N)), sizes={'y': N, 'x': N})
y = sc.broadcast(sc.arange('y', float(N)), sizes={'y': N, 'x': N})
z = sc.zeros_like(x)

a = sc.arange('a', float(N*N)).fold('a', sizes={'y':N, 'x':N})

da = sc.DataArray(data=a, coords={'x': x, 'y': y, 'z': z})
```

Using `scatter3d` produces
```Py
pp.scatter3d(da,
            x='x',
            y='y',
            z='z')
```
![Screenshot at 2023-02-06 11-22-07](https://user-images.githubusercontent.com/39047984/216947046-bd740445-d950-4577-a664-ddc29f6cda54.png)
where the first column `da['x', 0]` correctly ranges from 0 to ~2500.

However, using `figure3d` silently transposes the data (but not the dimensions!)
```Py
pp.figure3d(pp.input_node(da),
            x='x',
            y='y',
            z='z')
```
![Screenshot at 2023-02-06 11-22-21](https://user-images.githubusercontent.com/39047984/216947328-2154f452-bab0-454a-8cfa-aac354ee46fa.png)

Flattening before sending to `figure3d` fixes the issue
```Py
pp.figure3d(pp.input_node(da.flatten(to='dummy')),
            x='x',
            y='y',
            z='z')
```
![Screenshot at 2023-02-06 11-22-14](https://user-images.githubusercontent.com/39047984/216947735-defa70f8-b34c-4880-9760-cd454de3d845.png)


Discussion:

Patch:
diff --git a/src/plopp/backends/pythreejs/point_cloud.py b/src/plopp/backends/pythreejs/point_cloud.py
index bbafe62..5205460 100644
--- a/src/plopp/backends/pythreejs/point_cloud.py
+++ b/src/plopp/backends/pythreejs/point_cloud.py
@@ -8,6 +8,13 @@ import numpy as np
 import scipp as sc
 
 
+def _check_ndim(data):
+    if data.ndim != 1:
+        raise ValueError('PointCloud only accepts one dimensional data, '
+                         f'found {data.ndim} dimensions. You should flatten your data '
+                         '(using scipp.flatten) before sending it to the point cloud.')
+
+
 class PointCloud:
     """
     Artist to represent a three-dimensional point cloud/scatter plot.
@@ -41,6 +48,7 @@ class PointCloud:
         """
         import pythreejs as p3
 
+        _check_ndim(data)
         self._data = data
         self._x = x
         self._y = y
@@ -101,6 +109,7 @@ class PointCloud:
         new_values:
             New data to update the point cloud values from.
         """
+        _check_ndim(new_values)
         self._data = new_values
 
     def get_limits(self) -> Tuple[sc.Variable, sc.Variable, sc.Variable]:
diff --git a/src/plopp/functions/scatter3d.py b/src/plopp/functions/scatter3d.py
index aa7da7d..8d9bd61 100644
--- a/src/plopp/functions/scatter3d.py
+++ b/src/plopp/functions/scatter3d.py
@@ -40,10 +40,13 @@ def scatter3d(da: sc.DataArray,
         The data array containing the data and the coordinates.
     x:
         The name of the coordinate that is to be used for the X positions.
+        Default is 'x'.
     y:
         The name of the coordinate that is to be used for the Y positions.
+        Default is 'y'.
     z:
         The name of the coordinate that is to be used for the Z positions.
+        Default is 'z'.
     pos:
         The name of the vector coordinate that is to be used for the positions.
     norm:
@@ -86,6 +89,9 @@ def scatter3d(da: sc.DataArray,
             (z := f'{pos}.z'): da.meta[pos].fields.z
         }
     else:
+        x = x if x is not None else 'x'
+        y = y if y is not None else 'y'
+        z = z if z is not None else 'z'
         coords = {k: da.meta[k] for k in (x, y, z)}
 
     to_plot = sc.DataArray(data=da.data, masks=da.masks, coords=coords)
diff --git a/src/plopp/graphics/figscatter3d.py b/src/plopp/graphics/figscatter3d.py
index 48237cf..50c6ee8 100644
--- a/src/plopp/graphics/figscatter3d.py
+++ b/src/plopp/graphics/figscatter3d.py
@@ -52,9 +52,9 @@ class FigScatter3d(BaseFig):
 
     def __init__(self,
                  *nodes,
-                 x: str,
-                 y: str,
-                 z: str,
+                 x: str = 'x',
+                 y: str = 'y',
+                 z: str = 'z',
                  cmap: str = 'viridis',
                  mask_cmap: str = 'gray',
                  norm: Literal['linear', 'log'] = 'linear',
@@ -96,7 +96,6 @@ class FigScatter3d(BaseFig):
         draw:
             This argument is ignored for the 3d figure update.
         """
-
         xcoord = new_values.coords[self._x]
         ycoord = new_values.coords[self._y]
         zcoord = new_values.coords[self._z]


Test patch:
diff --git a/tests/backends/pythreejs/pythreejs_point_cloud_test.py b/tests/backends/pythreejs/pythreejs_point_cloud_test.py
index cd12217..653f7aa 100644
--- a/tests/backends/pythreejs/pythreejs_point_cloud_test.py
+++ b/tests/backends/pythreejs/pythreejs_point_cloud_test.py
@@ -89,3 +89,20 @@ def test_pixel_size_cannot_have_units_when_spatial_dimensions_have_different_uni
     # Ok if no unit supplied
     cloud = PointCloud(data=da, x='x', y='y', z='z', pixel_size=2.5)
     assert cloud.material.size == 2.5 * reference.material.size
+
+
+def test_creation_raises_when_data_is_not_1d():
+    da = scatter()
+    da2d = sc.broadcast(da, sizes={**da.sizes, **{'time': 10}})
+    with pytest.raises(ValueError,
+                       match='PointCloud only accepts one dimensional data'):
+        PointCloud(data=da2d, x='x', y='y', z='z')
+
+
+def test_update_raises_when_data_is_not_1d():
+    da = scatter()
+    cloud = PointCloud(data=da, x='x', y='y', z='z')
+    da2d = sc.broadcast(da, sizes={**da.sizes, **{'time': 10}})
+    with pytest.raises(ValueError,
+                       match='PointCloud only accepts one dimensional data'):
+        cloud.update(da2d)
diff --git a/tests/functions/scatter3d_test.py b/tests/functions/scatter3d_test.py
index f4a7403..65267f8 100644
--- a/tests/functions/scatter3d_test.py
+++ b/tests/functions/scatter3d_test.py
@@ -19,6 +19,11 @@ def test_scatter3d_from_xyz():
     pp.scatter3d(da, x='x', y='y', z='z')
 
 
+def test_scatter3d_from_xyz_using_defaults():
+    da = scatter()
+    pp.scatter3d(da)
+
+
 def test_scatter3d_raises_with_both_pos_and_xyz():
     da = scatter()
     with pytest.raises(ValueError, match=r'If pos \(position\) is defined, all of'):