Skip to content

Load NXdata auxiliary signals with distinct dimensions #306

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
CAOShurong wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Load NXdata auxiliary signals with distinct dimensions #306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/scippnexus/nxdata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ def _init_field_dims(self, name: str, field: Field | Group) -> None:
# len(dims).
shape = _squeeze_trailing(dims, field.dataset.shape)
field.sizes = dict(zip(dims, shape, strict=False))
elif name in self._aux_signals:

@SimonHeybrock SimonHeybrock Sep 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skipping the consistency check means the auxiliary signal keeps a generated dim label such as dim_0, and NXdata.sizes merges these into the group sizes. This can silently collide with the signal's dims. Example: signal with axes=['.'] (so its dim is dim_0, length 3) and frame_total with length 2:

  • group.sizes reports {'dim_0': 2}
  • group['dim_0', 2] raises IndexError, although it is a valid index for the signal
  • group['dim_0', 1] slices signal and frame_total with the same index, returning wrong data

On main this file is rejected with a fallback warning, which is the safer outcome.

# Auxiliary signals are allowed to have dimensions unrelated to the
# primary signal. Keep the field's fallback dimensions in that case.
return
elif self._valid:
s1 = self._signal.sizes
s2 = field.sizes
Expand Down Expand Up @@ -376,7 +380,13 @@ def _assemble_as_data(
signals = {self._signal_name: da}
signals.update(aux)
if all(isinstance(v, sc.Variable | sc.DataArray) for v in signals.values()):
return sc.Dataset(signals)
try:
return sc.Dataset(signals)
except sc.DimensionError:

@SimonHeybrock SimonHeybrock Sep 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Catching DimensionError here also hides genuine inconsistencies that are unrelated to "auxiliary signal on different dims", for example *_indices attributes that assign the same dim label with a different length. Previously these produced the fallback warning, now they load silently as a DataGroup.

# A Dataset requires matching item dimensionality. NeXus permits
# auxiliary signals on unrelated dimensions, so retain those as
# separate DataGroup entries instead.
pass
return sc.DataGroup(signals)
return da

Expand Down
20 changes: 20 additions & 0 deletions tests/nxdata_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,26 @@ def test_auxiliary_signal_causes_load_as_dataset(h5root) -> None:
assert_identical(data[...], sc.Dataset({'signal': signal, 'xx': aux}))


def test_auxiliary_signal_with_different_dims_loads_as_data_group(h5root) -> None:

@SimonHeybrock SimonHeybrock Sep 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This covers a bare NXdata with only frame_total. Two cases from the issue are not covered:

  • The group also contains a non-signal field on the frame dimension, such as reference_time (essreduce drops it for the same reason). With this PR such a group is still marked invalid and falls back with a warning, so the ESS monitor case would likely still fail.
  • An NXmonitor, which goes through _assemble_as_physical_component rather than _assemble_as_data.

signal = sc.array(dims=['tof'], unit='counts', values=[1, 2, 3])
frame_total = sc.array(dims=['frame'], unit='counts', values=[4, 5])
data = snx.create_class(h5root, 'data1', NXdata)
data.attrs['axes'] = signal.dims
data.attrs['signal'] = 'signal'
data.attrs['auxiliary_signals'] = ['frame_total']
snx.create_field(data, 'signal', signal)
snx.create_field(data, 'frame_total', frame_total)

loaded = snx.Group(data, definitions=snx.base_definitions())[...]

assert isinstance(loaded, sc.DataGroup)
assert_identical(loaded['signal'], sc.DataArray(signal))
assert_identical(
loaded['frame_total'],
sc.array(dims=['dim_0'], unit='counts', values=[4, 5]),
)


def test_NXlog_data_is_loaded_as_time_dependent_data_array(nxroot) -> None:
da = sc.DataArray(
data=sc.array(dims=['time'], unit='K', values=[1, 2, 3]),
Expand Down