diff --git a/src/schema/rules/checks/asl.yaml b/src/schema/rules/checks/asl.yaml index 1b42593212..2ae11ebeb0 100644 --- a/src/schema/rules/checks/asl.yaml +++ b/src/schema/rules/checks/asl.yaml @@ -196,21 +196,21 @@ ASLBackgroundSuppressionNumberPulses: - length(sidecar.BackgroundSuppressionPulseTime) == sidecar.BackgroundSuppressionNumberPulses # 181 -ASLTotalAcquiredVolumesASLContextLength: +ASLTotalAcquiredPairsASLContextLength: issue: code: TOTAL_ACQUIRED_VOLUMES_NOT_CONSISTENT message: | - The number of values for 'TotalAcquiredVolumes' for this file does not match number of + The number of values for 'TotalAcquiredPairs' for this file does not match number of volumes in the associated 'aslcontext.tsv'. - 'TotalAcquiredVolumes' is the original number of 3D volumes acquired for each volume defined in the + 'TotalAcquiredPairs' is the original number of 3D volumes acquired for each volume defined in the associated 'aslcontext.tsv'. level: warning selectors: - suffix == "asl" - type(associations.aslcontext) != "null" - - '"TotalAcquiredVolumes" in sidecar' + - '"TotalAcquiredPairs" in sidecar' checks: - - aslcontext.n_rows == sidecar.TotalAcquiredVolumes + - aslcontext.n_rows == sidecar.TotalAcquiredPairs # 184 PostLabelingDelayGreater: diff --git a/src/schema/rules/checks/micr.yaml b/src/schema/rules/checks/micr.yaml index bf2f4c55c3..7e78ab905a 100644 --- a/src/schema/rules/checks/micr.yaml +++ b/src/schema/rules/checks/micr.yaml @@ -10,17 +10,17 @@ PixelSizeInconsistent: selectors: - ome != null - sidecar.PixelSize != null - - sidecar.PixelSizeUnit != null + - sidecar.PixelSizeUnits != null checks: - | ome.PhysicalSizeX * 10 ** (-3 * index(["mm", "um", "nm"], ome.PhysicalSizeXUnit)) - == sidecar.PixelSize[0] * 10 ** (-3 * index(["mm", "um", "nm"], sidecar.PixelSizeUnit)) + == sidecar.PixelSize[0] * 10 ** (-3 * index(["mm", "um", "nm"], sidecar.PixelSizeUnits)) - | ome.PhysicalSizeY * 10 ** (-3 * index(["mm", "um", "nm"], ome.PhysicalSizeYUnit)) - == sidecar.PixelSize[1] * 10 ** (-3 * index(["mm", "um", "nm"], sidecar.PixelSizeUnit)) + == sidecar.PixelSize[1] * 10 ** (-3 * index(["mm", "um", "nm"], sidecar.PixelSizeUnits)) - | ome.PhysicalSizeZ * 10 ** (-3 * index(["mm", "um", "nm"], ome.PhysicalSizeZUnit)) - == sidecar.PixelSize[2] * 10 ** (-3 * index(["mm", "um", "nm"], sidecar.PixelSizeUnit)) + == sidecar.PixelSize[2] * 10 ** (-3 * index(["mm", "um", "nm"], sidecar.PixelSizeUnits)) # 227 InconsistentTiffExtension: diff --git a/src/schema/rules/checks/mri.yaml b/src/schema/rules/checks/mri.yaml index e2efc0216d..91a1520179 100644 --- a/src/schema/rules/checks/mri.yaml +++ b/src/schema/rules/checks/mri.yaml @@ -109,9 +109,9 @@ BolusCutOffDelayTimeNotMonotonicallyIncreasing: level: error selectors: - modality == "mri" - - sidecar.BolusCutoffDelayTime != null + - sidecar.BolusCutOffDelayTime != null checks: - - allequal(sorted(sidecar.BolusCutoffDelayTime), sidecar.BolusCutoffDelayTime) + - allequal(sorted(sidecar.BolusCutOffDelayTime), sidecar.BolusCutOffDelayTime) # 201 RepetitionTimePreparationNotConsistent: diff --git a/tools/schemacode/bidsschematools/tests/test_expressions.py b/tools/schemacode/bidsschematools/tests/test_expressions.py index 5532ef38ba..33eff61000 100644 --- a/tools/schemacode/bidsschematools/tests/test_expressions.py +++ b/tools/schemacode/bidsschematools/tests/test_expressions.py @@ -1,7 +1,21 @@ +from collections.abc import Mapping +from functools import singledispatch +from typing import Union + import pytest from pyparsing.exceptions import ParseException -from ..expressions import ASTNode, expression +from ..expressions import ( + Array, + ASTNode, + BinOp, + Element, + Function, + Property, + RightOp, + expression, +) +from ..types import Namespace def test_schema_expressions(schema_obj): @@ -77,3 +91,95 @@ def test_checks(schema_obj): def test_expected_failures(expr): with pytest.raises(ParseException): expression.parse_string(expr) + + +def walk_schema(schema_obj, predicate): + for key, value in schema_obj.items(): + if predicate(key, value): + yield key, value + if isinstance(value, Mapping): + for subkey, value in walk_schema(value, predicate): + yield f"{key}.{subkey}", value + + +def test_valid_sidecar_field(schema_obj): + """Check sidecar fields actually exist in the metadata listed in the schema. + + Test failures are usually due to typos. + """ + field_names = {field.name for key, field in schema_obj.objects.metadata.items()} + + for key, rule in walk_schema( + schema_obj.rules, lambda k, v: isinstance(v, Mapping) and v.get("selectors") + ): + for selector in rule["selectors"]: + ast = expression.parse_string(selector)[0] + for name in find_names(ast): + if name.startswith(("json.", "sidecar.")): + assert ( + name.split(".", 1)[1] in field_names + ), f"Bad field in selector: {name} ({key})" + for check in rule.get("checks", []): + ast = expression.parse_string(check)[0] + for name in find_names(ast): + if name.startswith(("json.", "sidecar.")): + assert ( + name.split(".", 1)[1] in field_names + ), f"Bad field in check: {name} ({key})" + + +def test_test_valid_sidecar_field(): + schema_obj = Namespace.build( + { + "objects": { + "metadata": { + "a": {"name": "a"}, + } + }, + "rules": {"myruleA": {"selectors": ["sidecar.a"], "checks": ["json.a == sidecar.a"]}}, + } + ) + test_valid_sidecar_field(schema_obj) + + schema_obj.objects.metadata.a["name"] = "b" + with pytest.raises(AssertionError): + test_valid_sidecar_field(schema_obj) + + +@singledispatch +def find_names(node: Union[ASTNode, str]): + # Walk AST nodes + if isinstance(node, BinOp): + yield from find_names(node.lh) + yield from find_names(node.rh) + elif isinstance(node, RightOp): + yield from find_names(node.rh) + elif isinstance(node, Array): + for element in node.elements: + yield from find_names(element) + elif isinstance(node, Element): + yield from find_names(node.name) + yield from find_names(node.index) + elif isinstance(node, (int, float)): + return + else: + raise TypeError(f"Unexpected node type: {node!r}") + + +@find_names.register +def find_function_names(node: Function): + yield node.name + for arg in node.args: + yield from find_names(arg) + + +@find_names.register +def find_property_name(node: Property): + # Properties are left-associative, so expand the left side + yield f"{next(find_names(node.name))}.{node.field}" + + +@find_names.register +def find_identifiers(node: str): + if not node.startswith(('"', "'")): + yield node