Repository: scipp/scippnexus

Issue:
`NXdata` group `@axes` attribute
How `@axes` should be encoded/interpreted has been discussed previously in #145 
   - there, I believe the discussion was about `@axes` applied to the `dataset` named 'DATA'

Comparing what the NeXus standard says regarding how the `@axes` attribute should be encoded when applied to the 'DATA' `dataset` and its containing `NXdata` group, there is some ambiguity.

Namely, notice that both use the term 'array'
> Defines the names of the coordinates (independent axes) for this data set as a colon-delimited array.
>
> &mdash; [`NXdata/data @axes`](https://manual.nexusformat.org/classes/base_classes/NXdata.html#nxdata-data-axes-attribute)

> When axes contains multiple strings, it must be saved as an actual array of strings and not a single comma separated string.
>
>  &mdash; [`NXdata @axes`](https://manual.nexusformat.org/classes/base_classes/NXdata.html#nxdata-axes-attribute) 

Where this becomes ambiguous, is in that HDF5 allows for array-valued attributes (which [h5py reads as numpy arrays](https://docs.h5py.org/en/stable/high/attr.html)), and `scippnexus` has interpreted both 'colon-delimited array' and 'actual array of strings' to specify a single string with axes names separated by literal ':' characters.


## Issue
If someone interprets 'actual array of strings' to mean 'array-valued attribute with string elements', then creates a NeXus HDF5 file, like the one attached; that file can not be opened by `scippnexus` due to the use of the `split` method here:
https://github.com/scipp/scippnexus/blob/58ffbbe67e90bf3341a7589e89e00e36859ba712/src/scippnexus/nxdata.py#L160

## Possible solution
Add a check that `self._signal_axes` is a `str` before attempting to use `.split`, or alternately a check if it is a numpy array, e.g., replacing the quoted line 160 by
```python
            self._signal_axes = tuple(self._signal_axes if isinstance(self._signal_axes, np.ndarray) else self._signal_axes.split(':'))
```

## Alternate solution
If the interpretation of 'actual array of strings' should be limited to 'colon-delimited array' like the 'DATA' dataset attribute, 
petition the NeXus committee to clarify their boxed-note for the group attribute.

Discussion:

Patch:
diff --git a/src/scippnexus/nxdata.py b/src/scippnexus/nxdata.py
index 55c1e5c..af23708 100644
--- a/src/scippnexus/nxdata.py
+++ b/src/scippnexus/nxdata.py
@@ -157,7 +157,11 @@ class NXdata(NXobject):
             None if self._signal is None else self._signal.attrs.get('axes')
         )
         if self._signal_axes is not None:
-            self._signal_axes = tuple(self._signal_axes.split(':'))
+            self._signal_axes = tuple(
+                self._signal_axes.split(':')
+                if isinstance(self._signal_axes, str)
+                else self._signal_axes
+            )
             # The standard says that the axes should be colon-separated, but some
             # files use comma-separated.
             if len(self._signal_axes) == 1 and self._signal.dataset.ndim > 1:
@@ -213,7 +217,11 @@ class NXdata(NXobject):
             group_dims = self._signal.dims
         else:
             if group_dims is not None:
-                shape = self._signal.dataset.shape
+                shape = (
+                    self._signal.dataset.shape
+                    if hasattr(self._signal, 'dataset')
+                    else self._signal.shape  # signal is a subgroup
+                )
                 # If we have explicit group dims, we can drop trailing 1s.
                 shape = _squeeze_trailing(group_dims, shape)
                 self._signal.sizes = dict(zip(group_dims, shape))


Test patch:
diff --git a/tests/nxmonitor_test.py b/tests/nxmonitor_test.py
index 94011e9..67b27a0 100644
--- a/tests/nxmonitor_test.py
+++ b/tests/nxmonitor_test.py
@@ -37,6 +37,25 @@ def test_dense_monitor(h5root):
     assert sc.identical(monitor[...]['data'], da)
 
 
+def test_monitor_with_nested_nxdata_signal(h5root):
+    da = sc.DataArray(
+        sc.array(dims=['xx', 'yy'], unit='m', values=[[1, 2, 3], [4, 5, 6]])
+    )
+    da.coords['xx'] = da.data['yy', 0]
+    monitor = snx.create_class(h5root, 'monitor', snx.NXmonitor)
+    date = '2024-01-01T00:00:00'
+    snx.create_field(monitor, 'start_time', date)
+    data = snx.create_class(monitor, 'data', snx.NXdata)
+    data.attrs['axes'] = da.dims
+    data.attrs['signal'] = 'signal'
+    snx.create_field(data, 'signal', da.data)
+    snx.create_field(data, 'xx', da.coords['xx'])
+    group = snx.Group(monitor, definitions=snx.base_definitions())
+    dg = group[...]
+    assert dg['start_time'] == date
+    assert sc.identical(dg['data'], da)
+
+
 def create_event_data_no_ids(group):
     group.create_field(
         'event_time_offset',