From e1b21b59203d2c5fbcb1d8e6755117f76e4bfd90 Mon Sep 17 00:00:00 2001 From: ggmarshall Date: Sun, 16 Aug 2026 04:24:27 +0200 Subject: [PATCH 1/2] fix(alias_table): walk parent groups iteratively and stop at the file root convert_parents_to_structs recursed on h5group.parent, but in h5py the root group is its own parent, so a file whose root already carries a datatype attribute sent the walk into infinite recursion. Rewrite the walk as a loop that terminates at '/', never invents a datatype on the root, and stops early when the child is already a registered member. Add a test aliasing into a new top-level group of a file with an annotated root. Co-Authored-By: Claude Fable 5 (cherry picked from commit 973d031af7233e613a890d20a82f3f2dff0b42fc) --- .../utils/alias_table.py | 64 +++++++++++-------- tests/test_utils_helpers.py | 28 ++++++++ 2 files changed, 65 insertions(+), 27 deletions(-) diff --git a/src/legenddataflowscripts/utils/alias_table.py b/src/legenddataflowscripts/utils/alias_table.py index df53e2e..30d1e88 100644 --- a/src/legenddataflowscripts/utils/alias_table.py +++ b/src/legenddataflowscripts/utils/alias_table.py @@ -7,7 +7,7 @@ def convert_parents_to_structs(h5group): - """Recursively annotate HDF5 parent groups with LGDO ``struct`` datatype attributes. + """Annotate HDF5 parent groups with LGDO ``struct`` datatype attributes. When a new alias (hard link) is created inside an HDF5 file the parent groups may not carry the ``datatype`` attribute expected by the LGDO reader. @@ -15,38 +15,48 @@ def convert_parents_to_structs(h5group): every ancestor group carries a ``datatype`` attribute of the form ``struct{child1,child2,…}``. + The root group is only updated if it already carries a ``datatype``; the + walk always stops there. + Parameters ---------- h5group : h5py.Group Leaf group whose parent hierarchy should be annotated. """ - child = h5group.name.split("/")[-1] - if h5group.parent.name != "/" and len(h5group.parent.attrs) == 0: - h5group.parent.attrs.update({"datatype": "struct{" + child + "}"}) - elif len(h5group.parent.attrs) > 0: - datatype = h5group.parent.attrs["datatype"] - if not (datatype.startswith("struct{") and datatype.endswith("}")): - msg = ( - f"cannot register alias {child!r}: parent group " - f"{h5group.parent.name!r} has non-struct datatype {datatype!r}" + # the walk must be iterative: in h5py the root group is its own parent, so + # recursing on ``h5group.parent`` never terminates once ``/`` is reached + while h5group.name != "/": + parent = h5group.parent + child = h5group.name.rsplit("/", 1)[-1] + + if "datatype" not in parent.attrs: + # never invent a datatype on the file root + if parent.name == "/": + return + parent.attrs.update({"datatype": "struct{" + child + "}"}) + else: + datatype = parent.attrs["datatype"] + if not (datatype.startswith("struct{") and datatype.endswith("}")): + msg = ( + f"cannot register alias {child!r}: parent group " + f"{parent.name!r} has non-struct datatype {datatype!r}" + ) + raise ValueError(msg) + # membership must be tested against the parsed field list, not by + # substring (e.g. 'raw' is a substring of 'raw_blind') + fields = [ + field.strip() + for field in datatype[len("struct{") : -1].split(",") + if field.strip() + ] + if child in fields: + # ancestors were annotated when this member was first added + return + parent.attrs.update( + {"datatype": "struct{" + ",".join([*fields, child]) + "}"} ) - raise ValueError(msg) - # membership must be tested against the parsed field list, not by - # substring (e.g. 'raw' is a substring of 'raw_blind') - fields = [ - field.strip() - for field in datatype[len("struct{") : -1].split(",") - if field.strip() - ] - if child in fields: - return - h5group.parent.attrs.update( - {"datatype": "struct{" + ",".join([*fields, child]) + "}"} - ) - else: - return - convert_parents_to_structs(h5group.parent) - return + + h5group = parent def alias_table(file: str | Path, mapping: str): diff --git a/tests/test_utils_helpers.py b/tests/test_utils_helpers.py index 0ba9060..53154d9 100644 --- a/tests/test_utils_helpers.py +++ b/tests/test_utils_helpers.py @@ -181,6 +181,34 @@ def test_alias_table_membership(tmp_path): assert f["det"].attrs["datatype"] == "struct{raw_blind,raw,raw2}" +def test_alias_table_annotated_root(tmp_path): + """Aliasing into a new top-level group of a file whose root is a struct.""" + lh5_file = tmp_path / "test.lh5" + with h5py.File(lh5_file, "w") as f: + for rawid, det in (("ch1027200", "V02160A"), ("ch1027201", "V02162B")): + grp = f.create_group(f"{rawid}/dsp") + grp.attrs["datatype"] = "table{a}" + grp.create_dataset("a", data=[1, 2, 3]) + f[rawid].attrs["datatype"] = "struct{dsp}" + # real LH5 files always carry a datatype on the root group; the parent + # walk must stop there instead of recursing on it forever + f["/"].attrs["datatype"] = "struct{ch1027200,ch1027201}" + + alias_table(lh5_file, {"ch1027200/dsp": "dsp/V02160A"}) + + with h5py.File(lh5_file) as f: + assert f["/"].attrs["datatype"] == "struct{ch1027200,ch1027201,dsp}" + assert f["dsp"].attrs["datatype"] == "struct{V02160A}" + assert list(f["dsp/V02160A/a"]) == [1, 2, 3] + + # a second alias extends the new group but leaves the root untouched + alias_table(lh5_file, {"ch1027201/dsp": "dsp/V02162B"}) + + with h5py.File(lh5_file) as f: + assert f["/"].attrs["datatype"] == "struct{ch1027200,ch1027201,dsp}" + assert f["dsp"].attrs["datatype"] == "struct{V02160A,V02162B}" + + def test_build_log_excepthook(tmp_path, monkeypatch): monkeypatch.setattr(sys, "stderr", sys.stderr) monkeypatch.setattr(sys, "excepthook", sys.excepthook) From 9ba1129ea6d0f6b9342579f6bdbd318476fe1bbe Mon Sep 17 00:00:00 2001 From: ggmarshall Date: Sun, 6 Sep 2026 21:39:01 +0200 Subject: [PATCH 2/2] style: drop unused loop variable in the alias regression test Co-Authored-By: Claude Fable 5 --- tests/test_utils_helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_utils_helpers.py b/tests/test_utils_helpers.py index 53154d9..da14c4d 100644 --- a/tests/test_utils_helpers.py +++ b/tests/test_utils_helpers.py @@ -185,7 +185,7 @@ def test_alias_table_annotated_root(tmp_path): """Aliasing into a new top-level group of a file whose root is a struct.""" lh5_file = tmp_path / "test.lh5" with h5py.File(lh5_file, "w") as f: - for rawid, det in (("ch1027200", "V02160A"), ("ch1027201", "V02162B")): + for rawid in ("ch1027200", "ch1027201"): grp = f.create_group(f"{rawid}/dsp") grp.attrs["datatype"] = "table{a}" grp.create_dataset("a", data=[1, 2, 3])