Skip to content

Move types to subpackages #607

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
jl-wynen wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Move types to subpackages #607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
461e3da
Remove frame monitor types from essreduce
jl-wynen May 28, 2026
18c74d8
Define monitor types in technique packages
jl-wynen May 28, 2026
64cc4ae
Define run types in technique packages
jl-wynen May 28, 2026
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
4 changes: 2 additions & 2 deletions packages/essdiffraction/src/ess/powder/types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@
LookupTableRelativeErrorThreshold = unwrap_t.LookupTableRelativeErrorThreshold

SampleRun = reduce_t.SampleRun
VanadiumRun = reduce_t.VanadiumRun
VanadiumRun = NewType("VanadiumRun", int)
EmptyCanRun = NewType("EmptyCanRun", int)

CaveMonitor = reduce_t.CaveMonitor
CaveMonitor = NewType("CaveMonitor", int)
BunkerMonitor = NewType("BunkerMonitor", int)

RunType = TypeVar("RunType", SampleRun, VanadiumRun, EmptyCanRun)
Expand Down
4 changes: 2 additions & 2 deletions packages/essdiffraction/tests/dream/workflows/dream_generic_workflow_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
import sciline
import scipp as sc
from ess import dream

from ess.reduce.nexus.types import (
from ess.powder.types import (
CaveMonitor,
DetectorBankSizes,
EmptyDetector,
Expand All @@ -16,6 +15,7 @@
RawDetector,
SampleRun,
)

from ess.reduce.nexus.types import NeXusName as NeXusMonitorName

bank_dims = {'wire', 'module', 'segment', 'strip', 'counter'}
Expand Down
41 changes: 14 additions & 27 deletions packages/essreduce/src/ess/reduce/nexus/types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ class ProductionInfo(snx.NXsource):
like ``SampleRun`` or ``BackgroundRun``.
"""

SampleRun = NewType('SampleRun', int)
Copy link
Copy Markdown
Member

@nvaytet nvaytet Jun 1, 2026

Choose a reason for hiding this comment

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

Why is the SampleRun special?
Shouldn't we remove all in the future, and sub-packages can decide if they want to name it SampleRun or something else?

Is there any code internally to essreduce that makes use of SampleRun or is it only in the tests/docs?

Copy link
Copy Markdown
Member Author

@jl-wynen jl-wynen Jun 1, 2026

Choose a reason for hiding this comment

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

It is special in how I and maybe we think about workflows. But that is not a good argument on its own.

There are some uses in live and polarization. Removing the ones in live might be tricky. But maybe that code should be in esslivedata anyway? And for polarization, there is this: #615

Copy link
Copy Markdown
Member

@nvaytet nvaytet Jun 1, 2026

Choose a reason for hiding this comment

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

I would have thought that everything in essreduce could just be RunType, unless I am forgetting something?

But maybe that code should be in esslivedata anyway?

Yes, I would vote to move it there.

Copy link
Copy Markdown
Member Author

@jl-wynen jl-wynen Jun 2, 2026

Choose a reason for hiding this comment

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

@SimonHeybrock ?

"""Sample run."""

# The following are legacy types and will be removed in the future.
# Use the types defined in technique packages instead.
BackgroundRun = NewType('BackgroundRun', int)
"""Background run such as a run with only a solvent which the sample is placed in."""
EmptyBeamRun = NewType('EmptyBeamRun', int)
Expand All @@ -66,8 +71,6 @@ class ProductionInfo(snx.NXsource):

It is used for reading the data from the transmission monitor.
"""
SampleRun = NewType('SampleRun', int)
"""Sample run."""
VanadiumRun = NewType('VanadiumRun', int)
"""Vanadium run."""

Expand Down Expand Up @@ -103,40 +106,24 @@ class TransmissionRun(Generic[ScatteringRunType]):
- :class:`VanadiumRun`
"""


# 1.2 Monitor types
IncidentMonitor = NewType('IncidentMonitor', int)
"""Incident monitor"""
TransmissionMonitor = NewType('TransmissionMonitor', int)
"""Transmission monitor"""
FrameMonitor0 = NewType('FrameMonitor0', int)
Copy link
Copy Markdown
Member

@nvaytet nvaytet Jun 1, 2026

Choose a reason for hiding this comment

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

Would there be any use in providing a default set of monitors? (Monitor1-4?)

Copy link
Copy Markdown
Member Author

@jl-wynen jl-wynen Jun 1, 2026

Choose a reason for hiding this comment

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

I don't think so. We shouldn't use them in workflows because they carry no meaning. So I think they would only be good for testing, and then we can easily define them in the tests.

"""Frame monitor number 0"""
FrameMonitor1 = NewType('FrameMonitor1', int)
"""Frame monitor number 1"""
FrameMonitor2 = NewType('FrameMonitor2', int)
"""Frame monitor number 2"""
FrameMonitor3 = NewType('FrameMonitor3', int)
"""Frame monitor number 3"""
CaveMonitor = NewType('CaveMonitor', int)
"""A monitor located in the instrument cave"""
MonitorType = TypeVar('MonitorType')
"""TypeVar for specifying what monitor some data belongs to.

This type must be constrained when used in a Sciline pipeline.
E.g., by passing ``monitor_types`` to :class:`ess.reduce.nexus.GenericNeXusWorkflow`.

ESSreduce provides the following but custom types can be used:

- :class:`IncidentMonitor`
- :class:`TransmissionMonitor`
- :class:`FrameMonitor0`
- :class:`FrameMonitor1`
- :class:`FrameMonitor2`
- :class:`FrameMonitor3`
- :class:`CaveMonitor`
"""

# The following are legacy types and will be removed in the future.
# Use the types defined in technique packages instead.
IncidentMonitor = NewType('IncidentMonitor', int)
"""Incident monitor"""
TransmissionMonitor = NewType('TransmissionMonitor', int)
"""Transmission monitor"""
CaveMonitor = NewType('CaveMonitor', int)
"""A monitor located in the instrument cave"""

# 1.3 Other component types
Component = TypeVar('Component')
"""A beamline component in a NeXus file."""
COMPONENT_CONSTRAINTS = (
Expand Down
8 changes: 5 additions & 3 deletions packages/essreduce/tests/nexus/nexus_loader_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from contextlib import contextmanager
from io import BytesIO
from pathlib import Path
from typing import Any
from typing import Any, NewType

import h5py as h5
import numpy as np
Expand All @@ -20,6 +20,8 @@

year_zero = sc.datetime('1970-01-01T00:00:00')

Monitor1 = NewType('Monitor1', int)


def _event_data_components() -> sc.DataGroup:
return sc.DataGroup(
Expand Down Expand Up @@ -282,7 +284,7 @@ def test_load_data_loads_expected_event_data(nexus_file, expected_bank12):
def test_load_data_loads_expected_histogram_data(nexus_file, expected_monitor):
histogram = nexus.load_data(
nexus_file,
component_name=nexus.types.NeXusName[nexus.types.CaveMonitor]('monitor'),
component_name=nexus.types.NeXusName[Monitor1]('monitor'),
)
sc.testing.assert_identical(histogram, expected_monitor)

Expand Down Expand Up @@ -514,7 +516,7 @@ def test_load_monitor(nexus_file, expected_monitor, entry_name, selection):
loc = NeXusLocationSpec(
filename=nexus_file,
entry_name=entry_name,
component_name=nexus.types.NeXusName[nexus.types.FrameMonitor1]('monitor'),
component_name=nexus.types.NeXusName[Monitor1]('monitor'),
)
if selection is not None:
loc.selection = selection
Expand Down
Loading
Loading