Skip to content

Commit 904abfc

Browse files
committed
Emitt warning when duplicate key with different value is found in OME metadata
1 parent 0aae9ce commit 904abfc

File tree

2 files changed

+57
-7
lines changed

2 files changed

+57
-7
lines changed

src/spatialdata_io/readers/macsima.py

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,14 @@ def _collect_map_annotation_values(ome: OME) -> dict[str, Any]:
354354
for k, v in value.items():
355355
if k not in merged:
356356
merged[k] = v
357+
else:
358+
# For instances where we have different values for the same repeated key, raise a warning.
359+
if v != merged[k]:
360+
warnings.warn(
361+
f"Found different value for {k}: {v}. The parser will only use the first found value, which is {merged[k]}!",
362+
UserWarning,
363+
stacklevel=2,
364+
)
357365

358366
return merged
359367

@@ -382,11 +390,16 @@ def _get_software_major_version(version: str) -> int:
382390
if not parts:
383391
raise ValueError("Could not extract major software version part from version string.")
384392

385-
return int(parts[0])
393+
major = int(parts[0])
394+
logger.debug(f"Found major software version {major}")
395+
396+
return major
386397

387398

388399
def _parse_v0_ome_metadata(ome: OME) -> dict[str, Any]:
389-
"""Parse ome."""
400+
"""Parse Legacy Format of OME Metadata (software version 0.x.x)."""
401+
logger.debug("Parsing OME metadata expecting version 0 format")
402+
390403
metadata: dict[str, Any] = {
391404
"name": None,
392405
"clone": None,
@@ -466,7 +479,9 @@ def _parse_v0_ome_metadata(ome: OME) -> dict[str, Any]:
466479

467480

468481
def _parse_v1_ome_metadata(ome: OME) -> dict[str, Any]:
469-
"""Your existing MACSima-style parser (1.x.x layout)."""
482+
"""Parse v1 format of OME metadata (software version 1.x.x)."""
483+
logger.debug("Parsing OME metadata expecting version 1 format")
484+
470485
metadata: dict[str, Any] = {
471486
"name": None,
472487
"clone": None,

tests/test_macsima.py

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -258,21 +258,56 @@ def test_cli_macsima(runner: CliRunner, dataset: str) -> None:
258258
_ = read_zarr(output_zarr)
259259

260260

261-
def test_collect_map_annotation_values_merges_all_keys_first_wins() -> None:
261+
def test_collect_map_annotation_values_with_no_duplicate_keys() -> None:
262262
ome = OME(
263263
structured_annotations=StructuredAnnotations(
264264
map_annotations=[
265265
MapAnnotation(value={"a": "1", "b": "2"}),
266-
MapAnnotation(value={"b": "99", "c": "3"}),
266+
MapAnnotation(value={"c": "3"}),
267+
]
268+
)
269+
)
270+
271+
result = _collect_map_annotation_values(ome)
272+
273+
assert result == {"a": "1", "b": "2", "c": "3"}
274+
275+
276+
def test_collect_map_annotations_values_with_duplicate_keys_identical_values() -> None:
277+
ome = OME(
278+
structured_annotations=StructuredAnnotations(
279+
map_annotations=[
280+
MapAnnotation(value={"a": "1", "b": "2"}),
281+
MapAnnotation(value={"b": "2", "c": "3"}),
267282
]
268283
)
269284
)
270285

271286
result = _collect_map_annotation_values(ome)
287+
# Key should only be returned once
288+
assert result == {"a": "1", "b": "2", "c": "3"}
289+
290+
291+
def test_collect_map_annotations_values_with_duplicate_keys_different_values() -> None:
292+
ome = OME(
293+
structured_annotations=StructuredAnnotations(
294+
map_annotations=[
295+
MapAnnotation(value={"a": "1", "b": "2"}),
296+
MapAnnotation(value={"b": "99", "c": "3"}),
297+
]
298+
)
299+
)
300+
import re
301+
302+
# The parser should throw a warning when duplicate keys with different values are found
303+
with pytest.warns(
304+
UserWarning,
305+
match=re.escape("Found different value for b: 99. The parser will only use the first found value, which is 2!"),
306+
):
307+
result = _collect_map_annotation_values(ome)
272308

309+
# The parser should return only the first found value.
273310
assert result == {"a": "1", "b": "2", "c": "3"}
274-
# ensure later duplicate "b" did not overwrite
275-
assert result["b"] == "2"
276311

277312

278313
def test_collect_map_annotation_values_handles_missing_sa_and_empty_list() -> None:

0 commit comments

Comments
 (0)