Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions src/ess/reduce/nexus/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
load_all_components,
load_component,
load_data,
load_from_path,
open_component_group,
open_nexus_file,
)
Expand All @@ -33,6 +34,7 @@
'load_all_components',
'load_component',
'load_data',
'load_from_path',
'open_component_group',
'open_nexus_file',
'types',
Expand Down
27 changes: 26 additions & 1 deletion src/ess/reduce/nexus/_nexus_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from contextlib import AbstractContextManager, contextmanager, nullcontext
from dataclasses import dataclass
from math import prod
from typing import TypeVar, cast
from typing import Any, TypeVar, cast

import scipp as sc
import scippnexus as snx
Expand Down Expand Up @@ -42,6 +42,31 @@ def __repr__(self) -> str:
NoLockingIfNeeded = NoLockingIfNeededType()


def load_from_path(
location: NeXusLocationSpec,
definitions: Mapping | NoNewDefinitionsType = NoNewDefinitions,
) -> Any:
"""Load a field or group from a NeXus file given its location.

Parameters
----------
location:
Location of the field within the NeXus file (filename, entry name, selection).
definitions:
Application definitions to use for the file.

Returns
-------
:
The loaded field (as a variable, data array, or raw python object) or group
(as a data group).
"""
with open_nexus_file(location.filename, definitions=definitions) as f:
entry = _unique_child_group(f, snx.NXentry, location.entry_name)
item = entry[location.component_name]
return item[location.selection]


def load_component(
location: NeXusLocationSpec,
*,
Expand Down
8 changes: 2 additions & 6 deletions src/ess/reduce/nexus/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -691,16 +691,12 @@ def LoadMonitorWorkflow(


def LoadDetectorWorkflow(
*,
run_types: Iterable[sciline.typing.Key],
monitor_types: Iterable[sciline.typing.Key],
Copy link
Member Author

@nvaytet nvaytet Nov 11, 2025

Choose a reason for hiding this comment

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

Not sure why we needed to have the monitor_types here, when we are loading detectors?
So I removed it.

*, run_types: Iterable[sciline.typing.Key]
) -> sciline.Pipeline:
"""Generic workflow for loading detector data from a NeXus file."""
wf = sciline.Pipeline(
(*_common_providers, *_detector_providers),
constraints=_gather_constraints(
run_types=run_types, monitor_types=monitor_types
),
constraints=_gather_constraints(run_types=run_types, monitor_types=[]),
)
wf[DetectorBankSizes] = DetectorBankSizes({})
wf[PreopenNeXusFile] = PreopenNeXusFile(False)
Expand Down
56 changes: 52 additions & 4 deletions tests/nexus/workflow_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
from pathlib import Path

import pytest
import sciline as sl
import scipp as sc
import scippnexus as snx
from scipp.testing import assert_identical

from ess.reduce.nexus import compute_component_position, workflow
from ess.reduce.nexus import compute_component_position, load_from_path, workflow
from ess.reduce.nexus.types import (
BackgroundRun,
Beamline,
Expand All @@ -21,6 +22,8 @@
Measurement,
MonitorType,
NeXusComponentLocationSpec,
NeXusFileSpec,
NeXusLocationSpec,
NeXusName,
NeXusTransformation,
PreopenNeXusFile,
Expand Down Expand Up @@ -577,7 +580,7 @@ def test_load_histogram_monitor_workflow(dream_coda_test_file: Path) -> None:


def test_load_detector_workflow(loki_tutorial_sample_run_60250: Path) -> None:
wf = LoadDetectorWorkflow(run_types=[SampleRun], monitor_types=[])
wf = LoadDetectorWorkflow(run_types=[SampleRun])
wf[Filename[SampleRun]] = loki_tutorial_sample_run_60250
wf[NeXusName[snx.NXdetector]] = 'larmor_detector'
da = wf.compute(RawDetector[SampleRun])
Expand All @@ -587,7 +590,7 @@ def test_load_detector_workflow(loki_tutorial_sample_run_60250: Path) -> None:


def test_load_histogram_detector_workflow(tbl_commissioning_orca_file: Path) -> None:
wf = LoadDetectorWorkflow(run_types=[SampleRun], monitor_types=[])
wf = LoadDetectorWorkflow(run_types=[SampleRun])
wf[Filename[SampleRun]] = tbl_commissioning_orca_file
wf[NeXusName[snx.NXdetector]] = 'orca_detector'
da = wf.compute(RawDetector[SampleRun])
Expand All @@ -600,7 +603,7 @@ def test_load_histogram_detector_workflow(tbl_commissioning_orca_file: Path) ->
def test_load_empty_histogram_detector_workflow(
tbl_commissioning_orca_file: Path,
) -> None:
wf = LoadDetectorWorkflow(run_types=[SampleRun], monitor_types=[])
wf = LoadDetectorWorkflow(run_types=[SampleRun])
wf[Filename[SampleRun]] = tbl_commissioning_orca_file
wf[NeXusName[snx.NXdetector]] = 'orca_detector'
da = wf.compute(EmptyDetector[SampleRun])
Expand Down Expand Up @@ -776,3 +779,48 @@ def assert_not_contains_type_arg(node: object, excluded: set[type]) -> None:
assert not any(
arg in excluded for arg in getattr(node, "__args__", ())
), f"Node {node} contains one of {excluded!r}"


def test_generic_nexus_workflow_load_custom_field_user_affiliation(
loki_tutorial_sample_run_60250: Path,
) -> None:
class UserAffiliation(sl.Scope[RunType, str], str):
"""User affiliation."""

def load_user_affiliation(
file: NeXusFileSpec[RunType], path: NeXusName[UserAffiliation[RunType]]
) -> UserAffiliation[RunType]:
return UserAffiliation[RunType](
load_from_path(NeXusLocationSpec(filename=file.value, component_name=path))
)

wf = GenericNeXusWorkflow(run_types=[SampleRun], monitor_types=[])
wf.insert(load_user_affiliation)
wf[Filename[SampleRun]] = loki_tutorial_sample_run_60250
# Path is relative to the top-level '/entry'
wf[NeXusName[UserAffiliation[SampleRun]]] = 'user_0/affiliation'
affiliation = wf.compute(UserAffiliation[SampleRun])
assert affiliation == 'ESS'


def test_generic_nexus_workflow_load_custom_group_user(
loki_tutorial_sample_run_60250: Path,
) -> None:
class UserInfo(sl.Scope[RunType, str], str):
"""User info."""

def load_user_info(
file: NeXusFileSpec[RunType], path: NeXusName[UserInfo[RunType]]
) -> UserInfo[RunType]:
return UserInfo[RunType](
load_from_path(NeXusLocationSpec(filename=file.value, component_name=path))
)

wf = GenericNeXusWorkflow(run_types=[SampleRun], monitor_types=[])
wf.insert(load_user_info)
wf[Filename[SampleRun]] = loki_tutorial_sample_run_60250
# Path is relative to the top-level '/entry'
wf[NeXusName[UserInfo]] = 'user_0'
user_info = wf.compute(UserInfo[SampleRun])
assert user_info['affiliation'] == 'ESS'
assert user_info['name'] == 'John Doe'
Copy link
Member

Choose a reason for hiding this comment

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

This should ideally be handled similarly to


and load one or more https://github.com/scipp/scippneutron/blob/3c49525dd89af68d375119c6f9072008f337dc6c/src/scippneutron/metadata/_model.py#L168

Does your code provide the basis for doing that?

Copy link
Member Author

Choose a reason for hiding this comment

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

So are you suggesting that instead of having

    class UserAffiliation(sl.Scope[RunType, str], str):
        """User affiliation."""

    def load_user_affiliation(
        file: NeXusFileSpec[RunType],
        path: NeXusName[UserAffiliation[RunType]],
    ) -> UserAffiliation[RunType]:
        return UserAffiliation[RunType](load_field(file, path))

I would need to create a pydantic model for the UserAffiliation with a from_nexus_entry classmethod, and then my load_user_affiliation would call load_metadata instead of load_field?

I guess I could but it feels a bit overkill?
I agree that for the proton charge, it might be worth it, because it will be used by multiple instruments.
But for something like an ExposureTime which is very specific to imaging, I feel it's too much effort?

In addition, I already felt it was annoying to have to make changes in essreduce so I could use them in essimaging, now I would have to first change scippneutron, then essreduce, then imaging...
I will say it again: should scippneutron and essreduce be merged?

Copy link
Member

@jl-wynen jl-wynen Nov 17, 2025

Choose a reason for hiding this comment

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

You don't need to make a model for UserAffiliation. We already have a Person model. All you need is a generic domain type for it (Like your UserAffiliation) and a provider to load it. It doesn't have to use a class method in ScippNeutron. And the code that needs the affiliation can extract that from a Person.

I think that the generic workflow should be able to load all users and provide a way to select one. (by index, path, or name, probably) Especially because this will be relevant to all workflows in some way.

And please don't misunderstand me, I think it is good to have tools for loading anything from the file in a simple way. But people related info is used in a lot of places. So I think we should have a common, robust, and flexible solution.

Copy link
Member Author

Choose a reason for hiding this comment

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

So, did I understand correctly that your objection was not adding load_field and load_group, but it was because I was using them to load the affiliation instead of using the load_metadata and Person model?

I just picked that field randomly, to illustrate that we can load something custom from the file. Is it better if I pick something else to load?

Copy link
Member

Choose a reason for hiding this comment

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

So, did I understand correctly that your objection was not adding load_field and load_group, but it was because I was using them to load the affiliation instead of using the load_metadata and Person model?

Correct. I am fine with the example and test case. My comment was only about checking whether this implementation is useful for implementing a loader for Person.
I'm now thinking that it probably isn't because we need to load an unknown number of users in general. So your functions here are unrelated.

Loading