Skip to content
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.

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

Improve and simplify loading of transformation chains #243

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
4b3c539
Preparations for new transformations module
SimonHeybrock Sep 26, 2024
433e17e
Make maybe_transformation injectable
SimonHeybrock Sep 27, 2024
135c28b
Fix new and old functionality
SimonHeybrock Sep 27, 2024
336c685
Add new tests
SimonHeybrock Sep 27, 2024
2ff9723
Remove try-except
SimonHeybrock Sep 27, 2024
e3274d0
Rename
SimonHeybrock Sep 27, 2024
3898850
Make tests runnable without extra data
SimonHeybrock Sep 27, 2024
cafcc0a
whitespace
SimonHeybrock Sep 27, 2024
05da5b4
Do not auto-conv to nested layout
SimonHeybrock Sep 27, 2024
da613cb
Some cleanup and docs
SimonHeybrock Sep 27, 2024
4bbc750
Add apply_to_transformations
SimonHeybrock Sep 27, 2024
6309d0d
Document module
SimonHeybrock Sep 27, 2024
25832c0
Revert "Make maybe_transformation injectable"
SimonHeybrock Sep 30, 2024
86fe7ec
Resolve chains and load as transformations.Transform
SimonHeybrock Sep 30, 2024
cc0c0b9
Add class DependsOn
SimonHeybrock Sep 30, 2024
1895830
Remove unused TransformationChainResolver
SimonHeybrock Sep 30, 2024
9f794c6
Remove unused arg
SimonHeybrock Sep 30, 2024
916f959
Simplify
SimonHeybrock Sep 30, 2024
e89b596
Update tests
SimonHeybrock Sep 30, 2024
30e81f8
Apply automatic formatting
pre-commit-ci-lite[bot] Sep 30, 2024
2a29c8c
grammar
SimonHeybrock Sep 30, 2024
08fceef
Cleanup
SimonHeybrock Sep 30, 2024
03b70ac
Remove unused
SimonHeybrock Sep 30, 2024
95d6c86
Move to common file
SimonHeybrock Sep 30, 2024
048e3ec
Docs and cleanup
SimonHeybrock Sep 30, 2024
00ee3cc
Forward the transformations
SimonHeybrock Sep 30, 2024
26c716d
Store chain in DependsOn subclass instead of adding new subgroup
SimonHeybrock Oct 1, 2024
020435d
Expose TransformationChain
SimonHeybrock Oct 1, 2024
6e94e00
Refactor
SimonHeybrock Oct 1, 2024
5d492b7
Docstring
SimonHeybrock Oct 1, 2024
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
22 changes: 20 additions & 2 deletions src/scippnexus/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,26 @@
create_class,
create_field,
)
from .field import Attrs, Field
from .field import Attrs, DependsOn, Field
from .file import File
from ._load import load
from .nexus_classes import *
from .nxtransformations import compute_positions, zip_pixel_offsets
from .nxtransformations import compute_positions, zip_pixel_offsets, TransformationChain

__all__ = [
'Attrs',
'DependsOn',
'Field',
'File',
'Group',
'NXobject',
'NexusStructureError',
'TransformationChain',
'base_definitions',
'compute_positions',
'create_class',
'create_field',
'load',
'typing',
'zip_pixel_offsets',
]
13 changes: 5 additions & 8 deletions src/scippnexus/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -409,16 +409,13 @@ def isclass(x):
# For a time-dependent transformation in NXtransformations, an NXlog may
# take the place of the `value` field. In this case, we need to read the
# properties of the NXlog group to make the actual transformation.
from .nxtransformations import maybe_resolve, maybe_transformation
from .nxtransformations import maybe_transformation, parse_depends_on_chain

if (
isinstance(dg, sc.DataGroup)
and (depends_on := dg.get('depends_on')) is not None
):
if (resolved := maybe_resolve(self['depends_on'], depends_on)) is not None:
dg['resolved_depends_on'] = resolved
if isinstance(dg, sc.DataGroup) and 'depends_on' in dg:
if (chain := parse_depends_on_chain(self, dg['depends_on'])) is not None:
dg['depends_on'] = chain

return maybe_transformation(self, value=dg, sel=sel)
return maybe_transformation(self, value=dg)

def _warn_fallback(self, e: Exception) -> None:
msg = (
Expand Down
32 changes: 19 additions & 13 deletions src/scippnexus/field.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,22 @@
from .base import Group


def depends_on_to_relative_path(depends_on: str, parent_path: str) -> str:
"""Replace depends_on paths with relative paths.
@dataclass
class DependsOn:
"""
Represents a depends_on reference in a NeXus file.

The parent (the full path within the NeXus file) is stored, as the value may be
relative or absolute, so having the path available after loading is essential.
"""

parent: str
value: str

After loading we will generally not have the same root so absolute paths
cannot be resolved after loading."""
if depends_on.startswith('/'):
return posixpath.relpath(depends_on, parent_path)
return depends_on
def absolute_path(self) -> str | None:
if self.value == '.':
return None
return posixpath.normpath(posixpath.join(self.parent, self.value))


def _is_time(obj):
Expand Down Expand Up @@ -161,7 +169,7 @@ def __getitem__(self, select: ScippIndex) -> Any | sc.Variable:
# If the variable is empty, return early
if np.prod(shape) == 0:
variable = self._maybe_datetime(variable)
return maybe_transformation(self, value=variable, sel=select)
return maybe_transformation(self, value=variable)

if self.dtype == sc.DType.string:
try:
Expand All @@ -170,10 +178,8 @@ def __getitem__(self, select: ScippIndex) -> Any | sc.Variable:
strings = self.dataset.asstr(encoding='latin-1')[index]
_warn_latin1_decode(self.dataset, strings, str(e))
variable.values = np.asarray(strings).flatten()
if self.dataset.name.endswith('depends_on') and variable.ndim == 0:
variable.value = depends_on_to_relative_path(
variable.value, self.dataset.parent.name
)
if self.dataset.name.endswith('/depends_on') and variable.ndim == 0:
return DependsOn(parent=self.dataset.parent.name, value=variable.value)
elif variable.values.flags["C_CONTIGUOUS"]:
# On versions of h5py prior to 3.2, a TypeError occurs in some cases
# where h5py cannot broadcast data with e.g. shape (20, 1) to a buffer
Expand All @@ -199,7 +205,7 @@ def __getitem__(self, select: ScippIndex) -> Any | sc.Variable:
else:
return variable.value
variable = self._maybe_datetime(variable)
return maybe_transformation(self, value=variable, sel=select)
return maybe_transformation(self, value=variable)

def _maybe_datetime(self, variable: sc.Variable) -> sc.Variable:
if _is_time(variable):
Expand Down
Loading