From b132a8e700c2cd03931f0c35d7a5fe49a21ce5f2 Mon Sep 17 00:00:00 2001 From: Dan Birman Date: Fri, 9 Oct 2026 13:46:18 -0700 Subject: [PATCH 1/5] refactor!: CoordinateSystem.NotApplicable --- docs/source/coordinate_systems.md | 14 ++++ examples/aibs_smartspim_instrument.py | 17 +---- examples/aibs_smartspim_procedures.py | 18 +---- examples/aind_smartspim_instrument.py | 17 +---- examples/barseq_acquisition.py | 2 + examples/barseq_instrument.py | 16 +---- examples/exaspim_acquisition.py | 1 + examples/exaspim_instrument.py | 18 +---- examples/fip_behavior_instrument.py | 17 +---- examples/fip_ophys_acquisition.py | 2 + examples/fip_ophys_instrument.py | 19 ++--- examples/isi_instrument.py | 17 +---- examples/ophys_acquisition.py | 2 + examples/ophys_procedures.py | 2 + examples/procedures.py | 3 +- examples/slap2_instrument.py | 17 +---- src/biodata_schema/components/coordinates.py | 7 +- .../components/specimen_procedures.py | 9 ++- .../components/subject_procedures.py | 5 +- src/biodata_schema/core/acquisition.py | 5 +- src/biodata_schema/core/instrument.py | 9 ++- src/biodata_schema/core/procedures.py | 5 +- src/biodata_schema/utils/merge.py | 6 +- src/biodata_schema/utils/validators.py | 69 +++++++++++++++++-- tests/test_coordinates.py | 55 +++++++++++++++ tests/test_examples.py | 25 +++++++ tests/test_imaging.py | 7 +- tests/test_instrument.py | 34 ++++----- tests/test_metadata.py | 4 +- tests/test_procedures.py | 62 +++++++++++++++-- tests/test_utils_validators.py | 61 ++++++++++++++++ 31 files changed, 368 insertions(+), 177 deletions(-) diff --git a/docs/source/coordinate_systems.md b/docs/source/coordinate_systems.md index 8d8e74fd2..624f32aff 100644 --- a/docs/source/coordinate_systems.md +++ b/docs/source/coordinate_systems.md @@ -26,6 +26,20 @@ CoordinateSystem( ) ``` +### Not Applicable + +When no global coordinate system applies, set `global_coordinate_system=CoordinateSystem.NotApplicable`. Note that this is different from using `global_coordinate_system=None` which will inherit from any parent coordinate systems. `"Not applicable"` should be used in situations where no transforms are present in an object. + +```{code} python +from biodata_schema.components.coordinates import CoordinateSystem +from biodata_schema.core.procedures import Procedures + +procedures = Procedures( + subject_name="12345", + global_coordinate_system=CoordinateSystem.NotApplicable, +) +``` + ### Origin An [Origin](biodata_models/coordinates.md#origin) is a point in space, often relative to the mouse's anatomy but it can also be a point on a device. The Origin defines the (0, 0, 0) coordinate in a coordinate system. Standard anatomical references are positions like Bregma or Lambda diff --git a/examples/aibs_smartspim_instrument.py b/examples/aibs_smartspim_instrument.py index 7dba480a2..ae0f649de 100644 --- a/examples/aibs_smartspim_instrument.py +++ b/examples/aibs_smartspim_instrument.py @@ -3,13 +3,11 @@ import argparse import datetime -from biodata_models.coordinates import AxisName, Direction, Origin from biodata_models.modalities import Modality from biodata_models.organizations import Organization -from biodata_models.units import SizeUnit from biodata_schema.components.connections import Connection -from biodata_schema.components.coordinates import Axis, CoordinateSystem +from biodata_schema.components.coordinates import CoordinateSystem from biodata_schema.components.devices import ( AdditionalImagingDevice, Detector, @@ -23,17 +21,6 @@ ) from biodata_schema.core.instrument import Instrument -SIPE_MONITOR_RTF = CoordinateSystem( - name="SIPE_MONITOR_RTF", - origin=Origin.FRONT_CENTER, - axis_unit=SizeUnit.MM, - axes=[ - Axis(name=AxisName.X, direction=Direction.LR), - Axis(name=AxisName.Y, direction=Direction.DU), - Axis(name=AxisName.Z, direction=Direction.BF), - ], -) - objective = Objective( name="TLX Objective", numerical_aperture=0.2, @@ -222,7 +209,7 @@ location="440", instrument_name="SmartSPIM2", modification_date=datetime.date(2023, 10, 4), - global_coordinate_system=SIPE_MONITOR_RTF, + global_coordinate_system=CoordinateSystem.NotApplicable, modalities=[Modality.SPIM], temperature_control=False, components=[ diff --git a/examples/aibs_smartspim_procedures.py b/examples/aibs_smartspim_procedures.py index f486995b2..0a07a18d2 100644 --- a/examples/aibs_smartspim_procedures.py +++ b/examples/aibs_smartspim_procedures.py @@ -3,26 +3,13 @@ import argparse from datetime import date -from biodata_models.coordinates import AxisName, Direction, Origin from biodata_models.organizations import Organization -from biodata_models.units import SizeUnit -from biodata_schema.components.coordinates import Axis, CoordinateSystem +from biodata_schema.components.coordinates import CoordinateSystem from biodata_schema.components.reagent import Reagent from biodata_schema.components.subject_procedures import Perfusion from biodata_schema.core import procedures -BREGMA_ARI = CoordinateSystem( - name="BREGMA_ARI", - origin=Origin.BREGMA, - axis_unit=SizeUnit.MM, - axes=[ - Axis(name=AxisName.AP, direction=Direction.PA), - Axis(name=AxisName.ML, direction=Direction.LR), - Axis(name=AxisName.SI, direction=Direction.SI), - ], -) - experimenters = ["John Smith"] specimen_name = "651286" @@ -43,7 +30,7 @@ start_date=date(2022, 11, 17), experimenters=["LAS"], ethics_review_id="2234", - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, procedures=[ Perfusion( protocol_id="dx.doi.org/10.17504/protocols.io.8epv51bejl1b/v6", @@ -95,6 +82,7 @@ all_procedures = procedures.Procedures( subject_name=specimen_name, + global_coordinate_system=CoordinateSystem.NotApplicable, subject_procedures=[ perfusion, ], diff --git a/examples/aind_smartspim_instrument.py b/examples/aind_smartspim_instrument.py index 3f25383eb..49b55da13 100644 --- a/examples/aind_smartspim_instrument.py +++ b/examples/aind_smartspim_instrument.py @@ -3,13 +3,11 @@ import argparse from datetime import date -from biodata_models.coordinates import AxisName, Direction, Origin from biodata_models.modalities import Modality from biodata_models.organizations import Organization -from biodata_models.units import SizeUnit from biodata_schema.components.connections import Connection -from biodata_schema.components.coordinates import Axis, CoordinateSystem +from biodata_schema.components.coordinates import CoordinateSystem from biodata_schema.components.devices import ( Device, Filter, @@ -24,17 +22,6 @@ Objective, ) -SPIM_RPI = CoordinateSystem( - name="SPIM_RPI", - origin=Origin.ORIGIN, - axis_unit=SizeUnit.MM, - axes=[ - Axis(name=AxisName.X, direction=Direction.LR), - Axis(name=AxisName.Y, direction=Direction.AP), - Axis(name=AxisName.Z, direction=Direction.SI), - ], -) - objective_1 = Objective( name="TLX Objective 1", numerical_aperture=0.2, @@ -264,7 +251,7 @@ location="440", instrument_name="SmartSPIM1", modification_date=date(2023, 10, 4), - global_coordinate_system=SPIM_RPI, + global_coordinate_system=CoordinateSystem.NotApplicable, modalities=[Modality.SPIM], components=[ scope, diff --git a/examples/barseq_acquisition.py b/examples/barseq_acquisition.py index 021ab4d55..baa9252e1 100644 --- a/examples/barseq_acquisition.py +++ b/examples/barseq_acquisition.py @@ -12,10 +12,12 @@ from biodata_models.modalities import Modality +from biodata_schema.components.coordinates import CoordinateSystem from biodata_schema.core.acquisition import Acquisition, ExternalDataStream acquisition = Acquisition( subject_name="123456", + global_coordinate_system=CoordinateSystem.NotApplicable, specimen_name=["123456_bar001", "123456_bar002"], acquisition_start_time=datetime(2025, 1, 1, 9, 0, 0, tzinfo=ZoneInfo("America/Los_Angeles")), acquisition_end_time=datetime(2025, 1, 31, 17, 0, 0, tzinfo=ZoneInfo("America/Los_Angeles")), diff --git a/examples/barseq_instrument.py b/examples/barseq_instrument.py index 4334d05cd..f5131b0cb 100644 --- a/examples/barseq_instrument.py +++ b/examples/barseq_instrument.py @@ -3,12 +3,11 @@ import argparse from datetime import date -from biodata_models.coordinates import AxisName, Direction, Origin from biodata_models.modalities import Modality from biodata_models.organizations import Organization from biodata_models.units import SizeUnit -from biodata_schema.components.coordinates import Axis, CoordinateSystem +from biodata_schema.components.coordinates import CoordinateSystem from biodata_schema.components.devices import ( BinMode, Camera, @@ -24,17 +23,6 @@ ) from biodata_schema.core.instrument import Instrument -IMAGE_XYZ = CoordinateSystem( - name="IMAGE_XYZ", - origin=Origin.ORIGIN, - axis_unit=SizeUnit.PX, - axes=[ - Axis(name=AxisName.X, direction=Direction.POS), - Axis(name=AxisName.Y, direction=Direction.POS), - Axis(name=AxisName.Z, direction=Direction.POS), - ], -) - objectives = [ Objective( name="20x Objective", @@ -228,7 +216,7 @@ location="243", instrument_name="Dogwood", modification_date=date(2024, 7, 9), - global_coordinate_system=IMAGE_XYZ, + global_coordinate_system=CoordinateSystem.NotApplicable, modalities=[Modality.BARSEQ], notes=( "BarSEQ imaging system with Nikon Ti2-E inverted microscope, X-Light V3 spinning disk confocal, " diff --git a/examples/exaspim_acquisition.py b/examples/exaspim_acquisition.py index 9a7524e9b..2bc01f02d 100644 --- a/examples/exaspim_acquisition.py +++ b/examples/exaspim_acquisition.py @@ -127,6 +127,7 @@ acq = Acquisition( experimenters=["John Smith"], + global_coordinate_system=SPIM_RPI, specimen_name="123456-123", subject_name="123456", instrument_name="exaSPIM1", diff --git a/examples/exaspim_instrument.py b/examples/exaspim_instrument.py index f14d14bba..9f44f4b0f 100644 --- a/examples/exaspim_instrument.py +++ b/examples/exaspim_instrument.py @@ -3,13 +3,12 @@ import argparse import datetime -from biodata_models.coordinates import AxisName, Direction, Origin from biodata_models.modalities import Modality from biodata_models.organizations import Organization -from biodata_models.units import FrequencyUnit, SizeUnit +from biodata_models.units import FrequencyUnit from biodata_schema.components.connections import Connection -from biodata_schema.components.coordinates import Axis, CoordinateSystem +from biodata_schema.components.coordinates import CoordinateSystem from biodata_schema.components.devices import ( AdditionalImagingDevice, Computer, @@ -25,17 +24,6 @@ ) from biodata_schema.core.instrument import Instrument -SPIM_RPI = CoordinateSystem( - name="SPIM_RPI", - origin=Origin.ORIGIN, - axis_unit=SizeUnit.MM, - axes=[ - Axis(name=AxisName.X, direction=Direction.LR), - Axis(name=AxisName.Y, direction=Direction.AP), - Axis(name=AxisName.Z, direction=Direction.SI), - ], -) - objectives = [ Objective( name="Custom Objective", @@ -284,7 +272,7 @@ instrument_name="exaSPIM1", modalities=[Modality.SPIM], modification_date=datetime.date(2023, 10, 4), - global_coordinate_system=SPIM_RPI, + global_coordinate_system=CoordinateSystem.NotApplicable, components=[ *objectives, *detectors, diff --git a/examples/fip_behavior_instrument.py b/examples/fip_behavior_instrument.py index 163ded50e..034362e3d 100644 --- a/examples/fip_behavior_instrument.py +++ b/examples/fip_behavior_instrument.py @@ -5,13 +5,13 @@ import argparse from datetime import date, datetime, timezone -from biodata_models.coordinates import AnatomicalRelative, AxisName, Direction, Origin +from biodata_models.coordinates import AnatomicalRelative from biodata_models.devices import CameraTarget from biodata_models.modalities import Modality from biodata_models.units import FrequencyUnit, PowerUnit, SizeUnit from biodata_schema.components.connections import Connection -from biodata_schema.components.coordinates import Axis, CoordinateSystem +from biodata_schema.components.coordinates import CoordinateSystem from biodata_schema.components.devices import ( Camera, CameraAssembly, @@ -38,17 +38,6 @@ from biodata_schema.components.measurements import Calibration from biodata_schema.core.instrument import Instrument -BREGMA_ARI = CoordinateSystem( - name="BREGMA_ARI", - origin=Origin.BREGMA, - axis_unit=SizeUnit.MM, - axes=[ - Axis(name=AxisName.AP, direction=Direction.PA), - Axis(name=AxisName.ML, direction=Direction.LR), - Axis(name=AxisName.SI, direction=Direction.SI), - ], -) - bonsai_software = Software(name="Bonsai", version="2.5") computer = Computer( @@ -399,7 +388,7 @@ instrument_name="FIP-Behavior", modification_date=date(2000, 1, 1), modalities=[Modality.BEHAVIOR, Modality.FIB], - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, components=[ camera1, camera2, diff --git a/examples/fip_ophys_acquisition.py b/examples/fip_ophys_acquisition.py index 2866829f3..2d29fa63a 100644 --- a/examples/fip_ophys_acquisition.py +++ b/examples/fip_ophys_acquisition.py @@ -19,6 +19,7 @@ TriggerType, ) from biodata_schema.components.connections import Connection +from biodata_schema.components.coordinates import CoordinateSystem from biodata_schema.components.identifiers import Code from biodata_schema.core.acquisition import ( Acquisition, @@ -336,6 +337,7 @@ # Create the acquisition object acquisition = Acquisition( + global_coordinate_system=CoordinateSystem.NotApplicable, experimenters=[ "Bryan MacLennan", "Kenta Hagihara", diff --git a/examples/fip_ophys_instrument.py b/examples/fip_ophys_instrument.py index 80c29e5ce..6dad08a11 100644 --- a/examples/fip_ophys_instrument.py +++ b/examples/fip_ophys_instrument.py @@ -3,29 +3,18 @@ import argparse from datetime import date, datetime, timezone -from biodata_models.coordinates import AnatomicalRelative, AxisName, Direction, Origin +from biodata_models.coordinates import AnatomicalRelative from biodata_models.modalities import Modality -from biodata_models.units import FrequencyUnit, PowerUnit, SizeUnit +from biodata_models.units import FrequencyUnit, PowerUnit import biodata_schema.components.devices as d import biodata_schema.core.instrument as r from biodata_schema.components.connections import Connection -from biodata_schema.components.coordinates import Axis, CoordinateSystem +from biodata_schema.components.coordinates import CoordinateSystem from biodata_schema.components.devices import Computer from biodata_schema.components.identifiers import Software from biodata_schema.components.measurements import Calibration -BREGMA_ARI = CoordinateSystem( - name="BREGMA_ARI", - origin=Origin.BREGMA, - axis_unit=SizeUnit.MM, - axes=[ - Axis(name=AxisName.AP, direction=Direction.PA), - Axis(name=AxisName.ML, direction=Direction.LR), - Axis(name=AxisName.SI, direction=Direction.SI), - ], -) - bonsai_software = Software(name="Bonsai", version="2.5") computer = Computer( @@ -343,7 +332,7 @@ instrument_name="FIP1", modification_date=date(2023, 10, 3), modalities=[Modality.FIB], - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, components=[ camera_assembly_1, camera_assembly_2, diff --git a/examples/isi_instrument.py b/examples/isi_instrument.py index 7f2ce0612..62e53e92c 100644 --- a/examples/isi_instrument.py +++ b/examples/isi_instrument.py @@ -14,7 +14,7 @@ import argparse from datetime import date -from biodata_models.coordinates import AnatomicalRelative, AxisName, Direction, Origin +from biodata_models.coordinates import AnatomicalRelative from biodata_models.devices import ( CameraChroma, CameraTarget, @@ -26,7 +26,7 @@ from biodata_models.organizations import Organization from biodata_models.units import SizeUnit -from biodata_schema.components.coordinates import Axis, CoordinateSystem +from biodata_schema.components.coordinates import CoordinateSystem from biodata_schema.components.devices import ( Camera, CameraAssembly, @@ -41,17 +41,6 @@ ) from biodata_schema.core.instrument import Instrument -BREGMA_ARI = CoordinateSystem( - name="BREGMA_ARI", - origin=Origin.BREGMA, - axis_unit=SizeUnit.MM, - axes=[ - Axis(name=AxisName.AP, direction=Direction.PA), - Axis(name=AxisName.ML, direction=Direction.LR), - Axis(name=AxisName.SI, direction=Direction.SI), - ], -) - acquisition_computer = Computer( name="Acquisition Computer", ) @@ -225,7 +214,7 @@ instrument_name="ISIV.1", modification_date=date(2026, 5, 15), modalities=[Modality.ISI], - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, temperature_control=True, notes="", components=[ diff --git a/examples/ophys_acquisition.py b/examples/ophys_acquisition.py index 8f7e5f82c..068b718b5 100644 --- a/examples/ophys_acquisition.py +++ b/examples/ophys_acquisition.py @@ -8,6 +8,7 @@ from biodata_schema.components.configs import Channel, DetectorConfig, LaserConfig, PatchCordConfig from biodata_schema.components.connections import Connection +from biodata_schema.components.coordinates import CoordinateSystem from biodata_schema.core.acquisition import ( Acquisition, AcquisitionSubjectDetails, @@ -39,6 +40,7 @@ a = Acquisition( experimenters=["Scientist Smith"], + global_coordinate_system=CoordinateSystem.NotApplicable, acquisition_start_time=t, acquisition_end_time=t, subject_name="652567", diff --git a/examples/ophys_procedures.py b/examples/ophys_procedures.py index 7b9362e5c..c509cf115 100644 --- a/examples/ophys_procedures.py +++ b/examples/ophys_procedures.py @@ -82,6 +82,7 @@ p = Procedures( subject_name="625100", + global_coordinate_system=CoordinateSystem.NotApplicable, subject_procedures=[ Surgery( start_date=t.date(), @@ -152,6 +153,7 @@ ), Surgery( start_date="2023-05-31", + global_coordinate_system=CoordinateSystem.NotApplicable, experimenters=["Scientist Smith"], ethics_review_id="2109", anaesthesia=Anaesthetic(anaesthetic_type="Isoflurane", duration=30, level=3), diff --git a/examples/procedures.py b/examples/procedures.py index ddf23152b..716a2c495 100644 --- a/examples/procedures.py +++ b/examples/procedures.py @@ -162,6 +162,7 @@ p = Procedures( subject_name="625100", + global_coordinate_system=CoordinateSystem.NotApplicable, subject_procedures=[ surgery1, Surgery( @@ -169,7 +170,7 @@ experimenters=["Scientist Smith"], ethics_review_id="2109", protocol_id="doi", - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, procedures=[ Perfusion( protocol_id="doi_of_protocol", diff --git a/examples/slap2_instrument.py b/examples/slap2_instrument.py index 6d39d1d23..fed083e4e 100644 --- a/examples/slap2_instrument.py +++ b/examples/slap2_instrument.py @@ -3,7 +3,7 @@ import argparse from datetime import datetime -from biodata_models.coordinates import AnatomicalRelative, AxisName, Direction, Origin +from biodata_models.coordinates import AnatomicalRelative from biodata_models.devices import CameraTarget, DetectorType, FilterType from biodata_models.harp_types import HarpDeviceType from biodata_models.modalities import Modality @@ -11,7 +11,7 @@ from biodata_models.units import SizeUnit, SpeedUnit from biodata_schema.components.connections import Connection -from biodata_schema.components.coordinates import Axis, CoordinateSystem +from biodata_schema.components.coordinates import CoordinateSystem from biodata_schema.components.devices import ( Camera, CameraAssembly, @@ -37,17 +37,6 @@ ) from biodata_schema.core.instrument import Instrument -BREGMA_ARI = CoordinateSystem( - name="BREGMA_ARI", - origin=Origin.BREGMA, - axis_unit=SizeUnit.MM, - axes=[ - Axis(name=AxisName.AP, direction=Direction.PA), - Axis(name=AxisName.ML, direction=Direction.LR), - Axis(name=AxisName.SI, direction=Direction.SI), - ], -) - computer_names = { "VCO": "w10dt714710", "SLAP2": "SLAP2-1-PC", @@ -358,7 +347,7 @@ location="443", instrument_name="SLAP2_1_VCO_1", modification_date=datetime.now().date(), - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, modalities=[Modality.SLAP2, Modality.BEHAVIOR, Modality.BEHAVIOR_VIDEOS], notes=( "Devices and connections not currently directly controlled or read out by" diff --git a/src/biodata_schema/components/coordinates.py b/src/biodata_schema/components/coordinates.py index 67e6caea6..274e11f46 100644 --- a/src/biodata_schema/components/coordinates.py +++ b/src/biodata_schema/components/coordinates.py @@ -1,7 +1,7 @@ """Classes to define device positions, orientations, and coordinates""" from enum import Enum -from typing import List, Optional +from typing import ClassVar, List, Literal, Optional from biodata_models.anatomy import AnatomyModel from biodata_models.atlas import AtlasName @@ -130,6 +130,8 @@ class NonlinearTransform(DataModel): class CoordinateSystem(DataModel): """Definition of a coordinate system""" + NotApplicable: ClassVar[Literal["Not applicable"]] = "Not applicable" + name: str = Field( ..., title="Name", description="Convention is to use _ etc" ) @@ -146,6 +148,9 @@ class CoordinateSystem(DataModel): ) +CoordinateSystemOrNotApplicable = CoordinateSystem | Literal["Not applicable"] + + class Atlas(CoordinateSystem): """Definition an atlas""" diff --git a/src/biodata_schema/components/specimen_procedures.py b/src/biodata_schema/components/specimen_procedures.py index 830afdc15..24ef39f49 100644 --- a/src/biodata_schema/components/specimen_procedures.py +++ b/src/biodata_schema/components/specimen_procedures.py @@ -15,7 +15,7 @@ DataModel, DiscriminatedList, ) -from biodata_schema.components.coordinates import Atlas, CoordinateSystem, Translation +from biodata_schema.components.coordinates import Atlas, CoordinateSystemOrNotApplicable, Translation from biodata_schema.components.identifiers import ProtocolListMixin from biodata_schema.components.reagent import ( FluorescentReagent, @@ -89,10 +89,13 @@ class Sectioning(DataModel): class PlanarSectioning(Sectioning): """Description of a sectioning procedure performed on the coronal, sagittal, or transverse/axial plane""" - global_coordinate_system: Optional[CoordinateSystem | Atlas] = Field( + global_coordinate_system: Optional[CoordinateSystemOrNotApplicable | Atlas] = Field( default=None, title="Sectioning global coordinate system", - description="Only required if different from the Procedures.global_coordinate_system", + description=( + "Only required if different from the Procedures.global_coordinate_system." + " Use 'Not applicable' to explicitly disable the inherited global frame." + ), ) sections: List[Union[Section, PlanarSection]] = Field( diff --git a/src/biodata_schema/components/subject_procedures.py b/src/biodata_schema/components/subject_procedures.py index 4d4db4197..13eb8e787 100644 --- a/src/biodata_schema/components/subject_procedures.py +++ b/src/biodata_schema/components/subject_procedures.py @@ -8,7 +8,7 @@ from pydantic import Field from biodata_schema.base import DataModel, DiscriminatedList -from biodata_schema.components.coordinates import CoordinateSystem, Translation +from biodata_schema.components.coordinates import CoordinateSystemOrNotApplicable, Translation from biodata_schema.components.identifiers import Code, ProtocolMixin from biodata_schema.components.injection_procedures import Injection from biodata_schema.components.surgery_procedures import ( @@ -122,12 +122,13 @@ class Surgery(ProtocolMixin, DataModel): workstation_id: Optional[str] = Field(default=None, title="Workstation ID") # Coordinate system - global_coordinate_system: Optional[CoordinateSystem] = Field( + global_coordinate_system: Optional[CoordinateSystemOrNotApplicable] = Field( default=None, title="Surgery global coordinate system", description=( "Only required when the Surgery.global_coordinate_system " "is different from the Procedures.global_coordinate_system" + ". Use 'Not applicable' to explicitly disable the inherited global frame." ), ) diff --git a/src/biodata_schema/core/acquisition.py b/src/biodata_schema/core/acquisition.py index 575cd1f6f..d7699a7e1 100644 --- a/src/biodata_schema/core/acquisition.py +++ b/src/biodata_schema/core/acquisition.py @@ -43,7 +43,7 @@ SpeakerConfig, ) from biodata_schema.components.connections import Connection -from biodata_schema.components.coordinates import CoordinateSystem +from biodata_schema.components.coordinates import CoordinateSystemOrNotApplicable from biodata_schema.components.identifiers import Code, ProtocolListMixin from biodata_schema.components.measurements import CALIBRATIONS, Maintenance from biodata_schema.components.reagent import Reagent @@ -448,12 +448,13 @@ def reject_non_iana_tz(cls, v): notes: Optional[str] = Field(default=None, title="Notes") # Coordinate system - global_coordinate_system: Optional[CoordinateSystem] = Field( + global_coordinate_system: Optional[CoordinateSystemOrNotApplicable] = Field( default=None, title="Global coordinate system", description=( "Origin and axis definitions for determining the configured position of devices during acquisition." " Required when coordinates are provided within the Acquisition" + ". Use 'Not applicable' when no global coordinate system applies." ), ) diff --git a/src/biodata_schema/core/instrument.py b/src/biodata_schema/core/instrument.py index 242671537..f953c8c02 100644 --- a/src/biodata_schema/core/instrument.py +++ b/src/biodata_schema/core/instrument.py @@ -9,7 +9,7 @@ from biodata_schema.base import DataCoreModel, DiscriminatedList, DraftRequirement from biodata_schema.components.connections import Connection -from biodata_schema.components.coordinates import CoordinateSystem +from biodata_schema.components.coordinates import CoordinateSystemOrNotApplicable from biodata_schema.components.devices import ( AdditionalImagingDevice, AirPuffDevice, @@ -104,10 +104,13 @@ class Instrument(DataCoreModel): ) # coordinate system - global_coordinate_system: CoordinateSystem = Field( + global_coordinate_system: CoordinateSystemOrNotApplicable = Field( ..., title="Global coordinate system", - description="Origin and axis definitions for determining the position of the instrument's components", + description=( + "Origin and axis definitions for determining the position of the instrument's components." + " Use 'Not applicable' when no global coordinate system applies." + ), ) # instrument details diff --git a/src/biodata_schema/core/procedures.py b/src/biodata_schema/core/procedures.py index 69d746707..6aa2ba620 100644 --- a/src/biodata_schema/core/procedures.py +++ b/src/biodata_schema/core/procedures.py @@ -5,7 +5,7 @@ from pydantic import Field, SkipValidation, model_validator from biodata_schema.base import DataCoreModel, DiscriminatedList -from biodata_schema.components.coordinates import CoordinateSystem +from biodata_schema.components.coordinates import CoordinateSystemOrNotApplicable from biodata_schema.components.specimen_procedures import SpecimenProcedure from biodata_schema.components.subject_procedures import ( GenericSubjectProcedure, @@ -38,12 +38,13 @@ class Procedures(DataCoreModel): ) # Coordinate system - global_coordinate_system: Optional[CoordinateSystem] = Field( + global_coordinate_system: Optional[CoordinateSystemOrNotApplicable] = Field( default=None, title="Global Coordinate System", description=( "Origin and axis definitions for determining the configured position of devices implanted during" " procedures. Required when coordinates are provided within the Procedures" + ". Use 'Not applicable' when no global coordinate system applies." ), ) diff --git a/src/biodata_schema/utils/merge.py b/src/biodata_schema/utils/merge.py index 92e0f213f..9ae2f2d68 100644 --- a/src/biodata_schema/utils/merge.py +++ b/src/biodata_schema/utils/merge.py @@ -108,11 +108,13 @@ def merge_notes(notes1: Optional[str], notes2: Optional[str]) -> Optional[str]: def merge_coordinate_systems(cs1: Optional[Any], cs2: Optional[Any]) -> Optional[Any]: - """Merge two coordinate system strings""" + """Merge coordinate systems, including explicit not-applicable values.""" if cs1 and cs2: if cs1 != cs2: - raise ValueError(f"Cannot merge differing coordinate systems: '{cs1.name}' and '{cs2.name}'") + name1 = getattr(cs1, "name", cs1) + name2 = getattr(cs2, "name", cs2) + raise ValueError(f"Cannot merge differing coordinate systems: '{name1}' and '{name2}'") return cs1 else: return cs1 if cs1 else cs2 diff --git a/src/biodata_schema/utils/validators.py b/src/biodata_schema/utils/validators.py index 9a5ac6c79..7fc8f3abb 100644 --- a/src/biodata_schema/utils/validators.py +++ b/src/biodata_schema/utils/validators.py @@ -157,6 +157,51 @@ def _iter_transforms(value): yield from _iter_transforms(item) +def _has_position_information(data, *, root: bool = True) -> bool: + """Find spatial transforms without borrowing positions from independent frames.""" + from biodata_schema.components.coordinates import ( + Affine, + CoordinateSystem, + NonlinearTransform, + Rotation, + Scale, + Translation, + ) + + if isinstance(data, (CoordinateSystem, Enum)): + return False + if not root and ( + getattr(data, "global_coordinate_system", None) is not None + or getattr(data, "coordinate_system", None) is not None + ): + return False + if isinstance(data, (Translation, Rotation, Scale, Affine, NonlinearTransform)): + return True + if isinstance(data, (list, tuple, dict)): + items = data.values() if isinstance(data, dict) else data + return any(_has_position_information(item, root=False) for item in items) + if not hasattr(data, "__dict__"): + return False + return any( + _has_position_information(value, root=False) + for name, value in vars(data).items() + if name != "dimensions" or not hasattr(data, "dimensions_unit") + ) + + +def _validate_coordinate_system_usage(data): + """Reject declared coordinate frames without any spatial position information.""" + from biodata_schema.components.coordinates import CoordinateSystem + + for field_name in ("global_coordinate_system", "local_coordinate_system", "coordinate_system"): + coordinate_system = getattr(data, field_name, None) + if isinstance(coordinate_system, CoordinateSystem) and not _has_position_information(data): + raise ValueError( + f"{field_name} is defined but no position information is present " + f"(object_type: {getattr(data, 'object_type', type(data).__name__)})" + ) + + def _check_transform_dimensions(transform, axis_count: int): """Check the parameters of a transform against its coordinate frame.""" from biodata_schema.components.coordinates import Affine, Rotation, Scale @@ -222,7 +267,11 @@ def _system_check_helper(data, coordinate_system_name: Optional[str], axis_count def recursive_coord_system_check( - data, coordinate_system_name: Optional[str], axis_count: Optional[int], local_coordinate_system=None + data, + coordinate_system_name: Optional[str], + axis_count: Optional[int], + local_coordinate_system=None, + global_not_applicable: bool = False, ): """Recursively validate coordinate system requirements for objects with transforms. @@ -250,19 +299,29 @@ def recursive_coord_system_check( Objects without transform components are not required to have coordinate systems. Nested global coordinate systems override the inherited global frame. Local frames apply to local-reference transforms; atlas coordinates carry their own frame. + NotApplicable clears the inherited global frame without skipping validation or + allowing a local frame to stand in for the explicitly absent global frame. + Declared frames require position information in their scope. Image dimensions + and positions in independent nested frames do not justify an unused frame. """ - from biodata_schema.components.coordinates import Affine, Rotation, Scale, Translation + from biodata_schema.components.coordinates import Affine, CoordinateSystem, Rotation, Scale, Translation if data is None or isinstance(data, Enum): return _cs = getattr(data, "global_coordinate_system", None) or getattr(data, "coordinate_system", None) - if _cs: + if _cs == CoordinateSystem.NotApplicable: + coordinate_system_name = None + axis_count = None + local_coordinate_system = None + global_not_applicable = True + elif _cs: coordinate_system_name = _cs.name axis_count = len(_cs.axes) local_coordinate_system = None + global_not_applicable = False local_coordinate_system = getattr(data, "local_coordinate_system", None) or local_coordinate_system - if axis_count is None and local_coordinate_system is not None: + if axis_count is None and local_coordinate_system is not None and not global_not_applicable: coordinate_system_name = local_coordinate_system.name axis_count = len(local_coordinate_system.axes) @@ -288,7 +347,9 @@ def recursive_coord_system_check( coordinate_system_name=coordinate_system_name, axis_count=axis_count, local_coordinate_system=local_coordinate_system, + global_not_applicable=global_not_applicable, ) + _validate_coordinate_system_usage(data) def recursive_get_named_objects(obj: Any) -> List[tuple]: diff --git a/tests/test_coordinates.py b/tests/test_coordinates.py index e020e00fb..dc1162414 100644 --- a/tests/test_coordinates.py +++ b/tests/test_coordinates.py @@ -3,12 +3,14 @@ import pytest from biodata_models.atlas import AtlasName from biodata_models.units import SizeUnit +from pydantic import TypeAdapter, ValidationError from biodata_schema.components.coordinates import ( Atlas, Axis, AxisName, CoordinateSystem, + CoordinateSystemOrNotApplicable, Direction, Handedness, Origin, @@ -17,6 +19,59 @@ RotationDirection, Translation, ) +from biodata_schema.components.specimen_procedures import PlanarSectioning +from biodata_schema.components.subject_procedures import Surgery +from biodata_schema.core.acquisition import Acquisition +from biodata_schema.core.instrument import Instrument +from biodata_schema.core.procedures import Procedures +from biodata_schema.utils.merge import merge_coordinate_systems +from tests.coordinate_systems import BREGMA_ARI + + +@pytest.mark.parametrize("model", [Instrument, Acquisition, Procedures, Surgery, PlanarSectioning]) +def test_not_applicable_global_coordinate_field(model): + """Every global frame accepts and round-trips only the explicit sentinel.""" + adapter = TypeAdapter(model.model_fields["global_coordinate_system"].annotation) + value = adapter.validate_python(CoordinateSystem.NotApplicable) + assert value == "Not applicable" + assert adapter.validate_json(adapter.dump_json(value)) == value + with pytest.raises(ValidationError): + adapter.validate_python("unknown") + schema = model.model_json_schema()["properties"]["global_coordinate_system"] + assert {"const": "Not applicable", "type": "string"} in schema["anyOf"] + assert model.model_fields["global_coordinate_system"].is_required() == (model is Instrument) + if model is not Instrument: + assert adapter.validate_python(None) is None + + +def test_not_applicable_is_not_a_coordinate_model_field(): + """The class constant does not change real coordinate-system objects.""" + assert "NotApplicable" not in CoordinateSystem.model_fields + assert "NotApplicable" not in BREGMA_ARI.model_dump() + adapter = TypeAdapter(CoordinateSystemOrNotApplicable) + assert adapter.validate_json(adapter.dump_json(BREGMA_ARI)) == BREGMA_ARI + + +@pytest.mark.parametrize( + "first,second,expected", + [ + (CoordinateSystem.NotApplicable, CoordinateSystem.NotApplicable, CoordinateSystem.NotApplicable), + (CoordinateSystem.NotApplicable, None, CoordinateSystem.NotApplicable), + (None, CoordinateSystem.NotApplicable, CoordinateSystem.NotApplicable), + ], +) +def test_merge_not_applicable_coordinate_systems(first, second, expected): + """Not-applicable frames merge like any other explicit frame.""" + assert merge_coordinate_systems(first, second) == expected + + +@pytest.mark.parametrize( + "first,second", [(CoordinateSystem.NotApplicable, BREGMA_ARI), (BREGMA_ARI, CoordinateSystem.NotApplicable)] +) +def test_merge_not_applicable_with_real_frame_fails(first, second): + """Contradictory frames raise a validation error, not an attribute error.""" + with pytest.raises(ValueError, match="Cannot merge differing coordinate systems"): + merge_coordinate_systems(first, second) class TestTranslationFrame: diff --git a/tests/test_examples.py b/tests/test_examples.py index 8623dd0de..ee9a1723d 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -21,6 +21,31 @@ def generated_examples(tmp_path_factory): class TestExamples: """tests for examples""" + @pytest.mark.parametrize( + "example_name", + [ + "aibs_smartspim_instrument", + "aind_smartspim_instrument", + "barseq_instrument", + "exaspim_instrument", + "fip_behavior_instrument", + "fip_ophys_instrument", + "isi_instrument", + "slap2_instrument", + "barseq_acquisition", + "ophys_acquisition", + "fip_ophys_acquisition", + "aibs_smartspim_procedures", + "ophys_procedures", + "procedures", + ], + ) + def test_position_free_global_frames_are_not_applicable(self, generated_examples, example_name): + """Examples without positions in their global scope explicitly mark it not applicable.""" + with (generated_examples / f"{example_name}.json").open() as example_file: + data = json.load(example_file) + assert data["global_coordinate_system"] == "Not applicable" + def test_examples_generated(self, generated_examples): """Test that each example file generates valid JSON.""" example_files = [ diff --git a/tests/test_imaging.py b/tests/test_imaging.py index cc783d753..d9a5b59fa 100644 --- a/tests/test_imaging.py +++ b/tests/test_imaging.py @@ -8,14 +8,13 @@ from pydantic import ValidationError from biodata_schema.components.configs import Image -from biodata_schema.components.coordinates import Affine, Rotation, Scale, Translation +from biodata_schema.components.coordinates import Affine, CoordinateSystem, Rotation, Scale, Translation from biodata_schema.components.devices import Laser, Objective, ScanningStage from biodata_schema.components.identifiers import Code from biodata_schema.core.acquisition import Acquisition from biodata_schema.core.instrument import Instrument from biodata_schema.core.processing import DataProcess, ProcessStage from examples.exaspim_acquisition import acq -from tests.coordinate_systems import BREGMA_ARI class TestImaging: @@ -62,7 +61,7 @@ def test_instrument_constructor(self): i = Instrument( instrument_name="room_exaSPIM1-1_20231004", modalities=[Modality.SPIM], - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, modification_date=datetime.now().date(), components=[objective, laser, scan_stage], ) @@ -76,7 +75,7 @@ def test_modality_spim_requires_components(self): instrument_name="room_exaSPIM1-1_20231004", modalities=[Modality.SPIM], modification_date=datetime(2020, 10, 10, 0, 0, 0).date(), - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, components=[], ) diff --git a/tests/test_instrument.py b/tests/test_instrument.py index 0c7c0c970..6fff293a1 100644 --- a/tests/test_instrument.py +++ b/tests/test_instrument.py @@ -13,7 +13,7 @@ from pydantic import ValidationError from biodata_schema.components.connections import Connection -from biodata_schema.components.coordinates import CoordinateSystem +from biodata_schema.components.coordinates import CoordinateSystem, Translation from biodata_schema.components.devices import ( Camera, CameraAssembly, @@ -352,7 +352,7 @@ def test_other_camera_target(self): instrument_name="123_EPHYS1-OPTO_20220101", modification_date=date(2020, 10, 10), modalities=[Modality.ECEPHYS, Modality.FIB], - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, components=[ *daqs, camera_no_target, @@ -388,7 +388,7 @@ def test_other_camera_target(self): instrument_name="123_EPHYS1-OPTO_20220101", modification_date=date(2020, 10, 10), modalities=[Modality.ECEPHYS, Modality.FIB], - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, components=[ *daqs, camera_no_target, @@ -429,7 +429,7 @@ def test_missing_connections(self): instrument_name="123_EPHYS1-OPTO_20220101", modification_date=date(2020, 10, 10), modalities=[Modality.ECEPHYS, Modality.FIB], - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, components=[ *daqs, *cameras, @@ -469,7 +469,7 @@ def test_missing_connections(self): instrument_name="123_EPHYS1-OPTO_20220101", modification_date=date(2020, 10, 10), modalities=[Modality.ECEPHYS, Modality.FIB], - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, components=[ *daqs, *cameras, @@ -513,7 +513,7 @@ def test_validator_modality_device_missing(self): Instrument( modalities=[Modality.from_abbreviation(modality_abbreviation)], instrument_name="123_EPHYS1-OPTO_20220101", - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, modification_date=date(2020, 10, 10), components=[], calibrations=[], @@ -528,7 +528,7 @@ def test_validator_modality_device_present(self): modalities=[Modality.from_abbreviation(modality_abbreviation)], instrument_name="123_EPHYS1-OPTO_20220101", modification_date=date(2020, 10, 10), - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, components=[ *daqs, *cameras, @@ -555,7 +555,7 @@ def test_serialize_modalities(self): instrument_instance_modality = Instrument.model_construct( instrument_name="123_EPHYS1-OPTO_20220101", modalities={Modality.ECEPHYS}, # Example with a valid Modality instance - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, ) instrument_json = instrument_instance_modality.model_dump_json() instrument_data = json.loads(instrument_json) @@ -589,6 +589,8 @@ def test_coordinate_validator(self): camera=camera, target=CameraTarget.BRAIN, relative_position=[AnatomicalRelative.SUPERIOR], + local_coordinate_system=BREGMA_ARI, + transform=[Translation(translation=[0, 0, 1])], lens=Lens(name="Lens A", manufacturer=Organization.OTHER, notes="Manufacturer unknown"), ) @@ -702,14 +704,14 @@ def test_duplicate_non_harp_device_components(self): instrument_name="test_inst", modification_date=date(2020, 10, 10), modalities=[Modality.ECEPHYS], - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, components=[Computer(name="Computer1")], ) inst2 = Instrument( instrument_name="test_inst", modification_date=date(2020, 10, 10), modalities=[Modality.ECEPHYS], - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, components=[Computer(name="Computer1")], ) @@ -737,14 +739,14 @@ def test_duplicate_harp_clock_generator_devices(self): instrument_name="test_inst", modification_date=date(2020, 10, 10), modalities=[Modality.ECEPHYS], - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, components=[harp_clock_gen], ) inst2 = Instrument( instrument_name="test_inst", modification_date=date(2020, 10, 10), modalities=[Modality.ECEPHYS], - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, components=[harp_clock_gen.model_copy(deep=True)], ) @@ -777,7 +779,7 @@ def test_duplicate_non_harp_device_with_clock_generator_attribute(self): instrument_name="test_inst", modification_date=date(2020, 10, 10), modalities=[Modality.BEHAVIOR], - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, components=[ harp_clock_gen, LickSpoutAssembly( @@ -797,7 +799,7 @@ def test_duplicate_non_harp_device_with_clock_generator_attribute(self): instrument_name="test_inst", modification_date=date(2020, 10, 10), modalities=[Modality.BEHAVIOR], - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, components=[ harp_non_clock_gen, LickSpoutAssembly( @@ -866,7 +868,7 @@ def test_connections_reject_software_names(self, endpoint): instrument_name="rig", modification_date=date(2026, 1, 1), modalities=[], - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, components=[camera], ) instrument = Instrument(**values) @@ -995,7 +997,7 @@ def test_validate_modalities_sorting(self): instrument_name="123_EPHYS1-OPTO_20220101", modification_date=date(2020, 10, 10), modalities=unsorted_modalities, - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, components=[ *daqs, *cameras, diff --git a/tests/test_metadata.py b/tests/test_metadata.py index d49b3b42e..58fd85169 100644 --- a/tests/test_metadata.py +++ b/tests/test_metadata.py @@ -14,6 +14,7 @@ from pydantic import ValidationError from biodata_schema.components.connections import Connection +from biodata_schema.components.coordinates import CoordinateSystem from biodata_schema.components.devices import EphysAssembly, EphysProbe, Laser, Manipulator from biodata_schema.components.identifiers import Code, Database, Person from biodata_schema.components.subject_procedures import TrainingProtocol @@ -34,7 +35,6 @@ from examples.processing import p as processing_example from examples.quality_control import q as quality_control_example from examples.subject import s as subject -from tests.coordinate_systems import BREGMA_ARI ephys_assembly = EphysAssembly( probes=[EphysProbe(probe_model="Neuropixels 1.0", name="Probe A")], @@ -193,7 +193,7 @@ def test_validate_instrument_acquisition_compatibility(self): instrument_name="123_EPHYS1_20220101", modalities=modalities, components=[ephys_assembly], - global_coordinate_system=BREGMA_ARI, + global_coordinate_system=CoordinateSystem.NotApplicable, ) with pytest.raises(ValidationError) as context: Metadata( diff --git a/tests/test_procedures.py b/tests/test_procedures.py index 1f74461d4..de3b718ce 100644 --- a/tests/test_procedures.py +++ b/tests/test_procedures.py @@ -20,7 +20,7 @@ from pydantic import ValidationError from biodata_schema.components.configs import CatheterConfig -from biodata_schema.components.coordinates import Origin, ReferenceCoordinateSystem, Translation +from biodata_schema.components.coordinates import CoordinateSystem, Origin, ReferenceCoordinateSystem, Translation from biodata_schema.components.devices import Catheter, Device from biodata_schema.components.injection_procedures import ( InjectionDynamics, @@ -39,7 +39,13 @@ SpecimenProcedure, ) from biodata_schema.components.subject_procedures import BrainInjection, Injection, Surgery -from biodata_schema.components.surgery_procedures import CatheterImplant, Craniotomy, CraniotomyType, GroundWireImplant +from biodata_schema.components.surgery_procedures import ( + CatheterImplant, + Craniotomy, + CraniotomyType, + GenericSurgeryProcedure, + GroundWireImplant, +) from biodata_schema.core.procedures import Procedures from biodata_schema.utils.exceptions import OneOfError from tests.coordinate_systems import BREGMA_ARI, BREGMA_RAS @@ -86,6 +92,46 @@ def test_unwrapped_injection_rejected(self): ], ) + @pytest.mark.parametrize( + "global_frame,surgery_frame,valid", + [ + (CoordinateSystem.NotApplicable, BREGMA_ARI, True), + (BREGMA_ARI, None, True), + (BREGMA_ARI, CoordinateSystem.NotApplicable, False), + (CoordinateSystem.NotApplicable, None, False), + ], + ) + def test_not_applicable_measured_coordinates(self, global_frame, surgery_frame, valid): + """Explicit absent surgery frames block inheritance, while real overrides work.""" + surgery = Surgery( + start_date=self.start_date, + global_coordinate_system=surgery_frame, + measured_coordinates={Origin.LAMBDA: Translation(translation=[-4.1, 0, 0])}, + procedures=[GenericSurgeryProcedure(description="Non-coordinate procedure")], + ) + values = dict(subject_name="12345", global_coordinate_system=global_frame, subject_procedures=[surgery]) + if valid: + procedures = Procedures(**values) + assert Procedures.model_validate_json(procedures.model_dump_json()) == procedures + else: + with pytest.raises(ValidationError, match="CoordinateSystem is required"): + Procedures(**values) + + def test_not_applicable_without_coordinates(self): + """A not-applicable frame is valid when there are no global coordinates.""" + procedures = Procedures( + subject_name="12345", + global_coordinate_system=CoordinateSystem.NotApplicable, + subject_procedures=[ + Surgery( + start_date=self.start_date, + global_coordinate_system=CoordinateSystem.NotApplicable, + procedures=[GenericSurgeryProcedure(description="Non-coordinate procedure")], + ) + ], + ) + assert Procedures.model_validate_json(procedures.model_dump_json()) == procedures + @patch("biodata_models.anatomy.MouseAnatomyLookup.get_by_name") def test_injection_material_check(self, mock_get_by_name): """Check for validation error when injection_materials is empty""" @@ -175,7 +221,6 @@ def test_injection_materials_list(self, mock_get_by_name): experimenters=["Mam Moth"], ethics_review_id="234", protocol_id="123", - global_coordinate_system=BREGMA_ARI, measured_coordinates={ Origin.BREGMA: Translation( translation=[0, 0, 0], @@ -810,15 +855,23 @@ def test_ground_wire_location_lookup(self, mock_get_by_name): def test_procedures_addition_coordinate_system_validation(self): """Test that Procedures addition raises error for different coordinate systems""" + surgery = Surgery( + start_date=self.start_date, + measured_coordinates={Origin.LAMBDA: Translation(translation=[-4.1, 0, 0])}, + procedures=[GenericSurgeryProcedure(description="Non-coordinate procedure")], + ) + # Create two procedures with different coordinate systems p1 = Procedures( subject_name="12345", global_coordinate_system=BREGMA_ARI, + subject_procedures=[surgery], ) p2 = Procedures( subject_name="12345", global_coordinate_system=BREGMA_RAS, # Different coordinate system + subject_procedures=[surgery], ) # Test that combining procedures with different coordinate systems raises ValueError @@ -833,8 +886,9 @@ def test_procedures_addition_coordinate_system_validation(self): p3 = Procedures( subject_name="12345", global_coordinate_system=BREGMA_ARI, # Same coordinate system as p1 + subject_procedures=[surgery], ) combined = p1 + p3 assert combined.global_coordinate_system == BREGMA_ARI - assert len(combined.subject_procedures) == 0 # Both started with empty procedures + assert len(combined.subject_procedures) == 2 diff --git a/tests/test_utils_validators.py b/tests/test_utils_validators.py index 53313229d..969611694 100644 --- a/tests/test_utils_validators.py +++ b/tests/test_utils_validators.py @@ -3,6 +3,7 @@ from datetime import date, datetime, timedelta, timezone from enum import Enum from pathlib import Path +from types import SimpleNamespace from typing import Annotated from unittest.mock import MagicMock, patch @@ -17,6 +18,7 @@ Affine, AtlasCoordinate, AtlasLibrary, + CoordinateSystem, ReferenceCoordinateSystem, Rotation, Scale, @@ -397,6 +399,65 @@ def test_measured_coordinate_dictionary(): recursive_coord_system_check({"lambda": Translation(translation=[-4.1, 0])}, "BREGMA_ARI", 3) +@pytest.mark.parametrize("field_name", ["global_coordinate_system", "local_coordinate_system", "coordinate_system"]) +def test_coordinate_system_requires_position_information(field_name): + """An unused real frame is rejected even when it only describes relative positions.""" + data = SimpleNamespace(**{field_name: BREGMA_ARI}, relative_position=["Anterior"], transform=[]) + with pytest.raises(ValueError, match="no position information"): + recursive_coord_system_check(data, None, None) + + +def test_coordinate_system_dimensions_are_not_positions(): + """Image dimensions must not justify an otherwise unused coordinate system.""" + data = SimpleNamespace( + global_coordinate_system=BREGMA_ARI, + image=SimpleNamespace(dimensions=Scale(scale=[100, 100, 100]), dimensions_unit=SizeUnit.PX), + ) + with pytest.raises(ValueError, match="no position information"): + recursive_coord_system_check(data, None, None) + + +def test_nested_frame_does_not_justify_unused_parent(): + """A nested independent frame has no positions in the parent frame.""" + data = SimpleNamespace( + global_coordinate_system=BREGMA_ARI, + nested=SimpleNamespace(global_coordinate_system=BREGMA_ARI, position=Translation(translation=[0, 0, 0])), + ) + with pytest.raises(ValueError, match="no position information"): + recursive_coord_system_check(data, None, None) + + +def test_measured_coordinates_justify_frame(): + """Positions in mappings nested in lists count as spatial information.""" + data = SimpleNamespace( + global_coordinate_system=BREGMA_ARI, + surgeries=[SimpleNamespace(measured_coordinates={"lambda": Translation(translation=[-4.1, 0, 0])})], + ) + recursive_coord_system_check(data, None, None) + + +def test_not_applicable_coordinate_frame(): + """An explicit absent global frame blocks inheritance but allows nested frames.""" + data = SimpleNamespace(global_coordinate_system=CoordinateSystem.NotApplicable) + recursive_coord_system_check(data, "BREGMA_ARI", 3) + data.transform = Translation(translation=[0, 0, 0]) + with pytest.raises(ValueError, match="CoordinateSystem is required"): + recursive_coord_system_check(data, "BREGMA_ARI", 3) + data.local_coordinate_system = BREGMA_ARI + with pytest.raises(ValueError, match="CoordinateSystem is required"): + recursive_coord_system_check(data, "BREGMA_ARI", 3) + data.transform = Translation(translation=[0, 0, 0], reference_coordinate_system=ReferenceCoordinateSystem.LOCAL) + recursive_coord_system_check(data, "BREGMA_ARI", 3) + data.local_coordinate_system = None + data.transform = SimpleNamespace( + global_coordinate_system=BREGMA_ARI, + transform=Translation(translation=[0, 0, 0]), + ) + recursive_coord_system_check(data, None, None) + data.transform = AtlasCoordinate(translation=[0, 0, 0], coordinate_system=AtlasLibrary.CCFv3_10um) + recursive_coord_system_check(data, None, None) + + def test_local_and_atlas_coordinate_frames(): """Local positions and atlas coordinates use their own frames.""" manipulator = ManipulatorConfig( From 517377d6563dad7c4d2047ad4a15c638268433fe Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 9 Oct 2026 20:48:42 +0000 Subject: [PATCH 2/5] update docs --- docs/source/acquisition.md | 2 +- docs/source/components/specimen_procedures.md | 2 +- docs/source/components/subject_procedures.md | 2 +- docs/source/instrument.md | 2 +- docs/source/procedures.md | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/source/acquisition.md b/docs/source/acquisition.md index 470e69cb4..0cef1bcf9 100644 --- a/docs/source/acquisition.md +++ b/docs/source/acquisition.md @@ -87,7 +87,7 @@ while the StimulusEpoch represents all stimuli being presented. | `instrument_name` | `Optional[str]` | Instrument name (Should match the Instrument.instrument_name. Required when instrument metadata is available.) | | `acquisition_type` | `str` | Acquisition type (Descriptive string detailing the type of acquisition, should be consistent across similar acquisitions for the same experiment.) | | `notes` | `Optional[str]` | Notes | -| `global_coordinate_system` | Optional[[CoordinateSystem](components/coordinates.md#coordinatesystem)] | Global coordinate system (Origin and axis definitions for determining the configured position of devices during acquisition. Required when coordinates are provided within the Acquisition) | +| `global_coordinate_system` | [CoordinateSystem](components/coordinates.md#coordinatesystem) or "Not applicable" or NoneType | Global coordinate system (Origin and axis definitions for determining the configured position of devices during acquisition. Required when coordinates are provided within the Acquisition. Use 'Not applicable' when no global coordinate system applies.) | | `calibrations` | List[[Calibration](components/measurements.md#calibration) or [VolumeCalibration](components/measurements.md#volumecalibration) or [PowerCalibration](components/measurements.md#powercalibration)] | Calibrations (List of calibration measurements taken prior to acquisition.) | | `maintenance` | List[[Maintenance](components/measurements.md#maintenance)] | Maintenance (List of maintenance on instrument prior to acquisition.) | | `data_streams` | List[[DataStream](acquisition.md#datastream) or [ExternalDataStream](acquisition.md#externaldatastream)] | Data streams (A data stream is a collection of devices that are acquiring data simultaneously. Each acquisition can include multiple streams. Streams should be split when configurations are changed. Use ExternalDataStream for acquisitions where instrument metadata is unavailable.) | diff --git a/docs/source/components/specimen_procedures.md b/docs/source/components/specimen_procedures.md index 9cecc4d46..e466790df 100644 --- a/docs/source/components/specimen_procedures.md +++ b/docs/source/components/specimen_procedures.md @@ -52,7 +52,7 @@ Description of a sectioning procedure performed on the coronal, sagittal, or tra | Field | Type | Title (Description) | |-------|------|-------------| -| `global_coordinate_system` | [CoordinateSystem](coordinates.md#coordinatesystem) or [Atlas](coordinates.md#atlas) or NoneType | Sectioning global coordinate system (Only required if different from the Procedures.global_coordinate_system) | +| `global_coordinate_system` | [CoordinateSystem](coordinates.md#coordinatesystem) or "Not applicable" or [Atlas](coordinates.md#atlas) or NoneType | Sectioning global coordinate system (Only required if different from the Procedures.global_coordinate_system. Use 'Not applicable' to explicitly disable the inherited global frame.) | | `sections` | List[[Section](#section) or [PlanarSection](#planarsection)] | Planar sections (Use PlanarSection for new implementations) | | `section_orientation` | [SectionOrientation](#sectionorientation) | Sectioning orientation | diff --git a/docs/source/components/subject_procedures.md b/docs/source/components/subject_procedures.md index 61a285411..c133a256f 100644 --- a/docs/source/components/subject_procedures.md +++ b/docs/source/components/subject_procedures.md @@ -61,7 +61,7 @@ Description of subject procedures performed at one time | `weight_unit` | [MassUnit](../biodata_models/units.md#massunit) | Weight unit | | `anaesthesia` | Optional[[Anaesthetic](surgery_procedures.md#anaesthetic)] | Anaesthesia | | `workstation_id` | `Optional[str]` | Workstation ID | -| `global_coordinate_system` | Optional[[CoordinateSystem](coordinates.md#coordinatesystem)] | Surgery global coordinate system (Only required when the Surgery.global_coordinate_system is different from the Procedures.global_coordinate_system) | +| `global_coordinate_system` | [CoordinateSystem](coordinates.md#coordinatesystem) or "Not applicable" or NoneType | Surgery global coordinate system (Only required when the Surgery.global_coordinate_system is different from the Procedures.global_coordinate_system. Use 'Not applicable' to explicitly disable the inherited global frame.) | | `measured_coordinates` | Optional[Dict[[Origin](../biodata_models/coordinates.md#origin), [Translation](coordinates.md#translation)]] | Measured coordinates (Coordinates measured during the procedure, for example Bregma and Lambda) | | `procedures` | List[[CatheterImplant](surgery_procedures.md#catheterimplant) or [Craniotomy](surgery_procedures.md#craniotomy) or [DeviceImplant](surgery_procedures.md#deviceimplant) or [ProbeImplant](surgery_procedures.md#probeimplant) or [Headframe](surgery_procedures.md#headframe) or [BrainInjection](surgery_procedures.md#braininjection) or [Injection](injection_procedures.md#injection) or [MyomatrixInsertion](surgery_procedures.md#myomatrixinsertion) or [GenericSurgeryProcedure](surgery_procedures.md#genericsurgeryprocedure) or [Perfusion](surgery_procedures.md#perfusion) or [SampleCollection](surgery_procedures.md#samplecollection)] | Procedures | | `notes` | `Optional[str]` | Notes | diff --git a/docs/source/instrument.md b/docs/source/instrument.md index 1a0f0d61d..c19e68b48 100644 --- a/docs/source/instrument.md +++ b/docs/source/instrument.md @@ -60,7 +60,7 @@ Description of an instrument | `modification_date` | `datetime.date` | Date of modification (Date of the last change to the instrument, hardware addition/removal, calibration, etc.) | | `modalities` | List[[Modality](biodata_models/modalities.md#modality)] | Modalities (List of all possible modalities that the instrument is capable of acquiring) | | `calibrations` | Optional[List[[Calibration](components/measurements.md#calibration) or [VolumeCalibration](components/measurements.md#volumecalibration) or [PowerCalibration](components/measurements.md#powercalibration)]] | Calibrations (List of calibration measurements takend during instrument setup and maintenance) | -| `global_coordinate_system` | [CoordinateSystem](components/coordinates.md#coordinatesystem) | Global coordinate system (Origin and axis definitions for determining the position of the instrument's components) | +| `global_coordinate_system` | [CoordinateSystem](components/coordinates.md#coordinatesystem) or "Not applicable" | Global coordinate system (Origin and axis definitions for determining the position of the instrument's components. Use 'Not applicable' when no global coordinate system applies.) | | `temperature_control` | `Optional[bool]` | Temperature control (Does the instrument maintain a constant temperature?) | | `notes` | `Optional[str]` | Notes | | `connections` | List[[Connection](components/connections.md#connection)] | Connections (List of all connections between devices in the instrument) | diff --git a/docs/source/procedures.md b/docs/source/procedures.md index 29e669545..d2ade6a31 100644 --- a/docs/source/procedures.md +++ b/docs/source/procedures.md @@ -29,5 +29,5 @@ Description of all procedures performed on a subject, including surgeries, injec | `subject_name` | `str` | Subject name (Unique name for the subject of data acquisition) | | `subject_procedures` | List[[Surgery](components/subject_procedures.md#surgery) or [NonSurgicalInjection](components/subject_procedures.md#nonsurgicalinjection) or [TrainingProtocol](components/subject_procedures.md#trainingprotocol) or [WaterRestriction](components/subject_procedures.md#waterrestriction) or [GenericSubjectProcedure](components/subject_procedures.md#genericsubjectprocedure)] | Subject Procedures (Procedures performed on a live subject) | | `specimen_procedures` | List[[SpecimenProcedure](components/specimen_procedures.md#specimenprocedure)] | Specimen Procedures (Procedures performed on tissue extracted after perfusion) | -| `global_coordinate_system` | Optional[[CoordinateSystem](components/coordinates.md#coordinatesystem)] | Global Coordinate System (Origin and axis definitions for determining the configured position of devices implanted during procedures. Required when coordinates are provided within the Procedures) | +| `global_coordinate_system` | [CoordinateSystem](components/coordinates.md#coordinatesystem) or "Not applicable" or NoneType | Global Coordinate System (Origin and axis definitions for determining the configured position of devices implanted during procedures. Required when coordinates are provided within the Procedures. Use 'Not applicable' when no global coordinate system applies.) | | `notes` | `Optional[str]` | Notes | From c6e4219b33e34c71ac1aa7cf7a0a718646fe19db Mon Sep 17 00:00:00 2001 From: Dan Birman Date: Fri, 9 Oct 2026 13:48:52 -0700 Subject: [PATCH 3/5] chore: shorten shortcut name --- src/biodata_schema/components/coordinates.py | 2 +- src/biodata_schema/components/specimen_procedures.py | 4 ++-- src/biodata_schema/components/subject_procedures.py | 4 ++-- src/biodata_schema/core/acquisition.py | 4 ++-- src/biodata_schema/core/instrument.py | 4 ++-- src/biodata_schema/core/procedures.py | 4 ++-- tests/test_coordinates.py | 4 ++-- 7 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/biodata_schema/components/coordinates.py b/src/biodata_schema/components/coordinates.py index 274e11f46..6f3042d63 100644 --- a/src/biodata_schema/components/coordinates.py +++ b/src/biodata_schema/components/coordinates.py @@ -148,7 +148,7 @@ class CoordinateSystem(DataModel): ) -CoordinateSystemOrNotApplicable = CoordinateSystem | Literal["Not applicable"] +CoordinateSystemOrNA = CoordinateSystem | Literal["Not applicable"] class Atlas(CoordinateSystem): diff --git a/src/biodata_schema/components/specimen_procedures.py b/src/biodata_schema/components/specimen_procedures.py index 24ef39f49..bbd288dce 100644 --- a/src/biodata_schema/components/specimen_procedures.py +++ b/src/biodata_schema/components/specimen_procedures.py @@ -15,7 +15,7 @@ DataModel, DiscriminatedList, ) -from biodata_schema.components.coordinates import Atlas, CoordinateSystemOrNotApplicable, Translation +from biodata_schema.components.coordinates import Atlas, CoordinateSystemOrNA, Translation from biodata_schema.components.identifiers import ProtocolListMixin from biodata_schema.components.reagent import ( FluorescentReagent, @@ -89,7 +89,7 @@ class Sectioning(DataModel): class PlanarSectioning(Sectioning): """Description of a sectioning procedure performed on the coronal, sagittal, or transverse/axial plane""" - global_coordinate_system: Optional[CoordinateSystemOrNotApplicable | Atlas] = Field( + global_coordinate_system: Optional[CoordinateSystemOrNA | Atlas] = Field( default=None, title="Sectioning global coordinate system", description=( diff --git a/src/biodata_schema/components/subject_procedures.py b/src/biodata_schema/components/subject_procedures.py index 13eb8e787..e4a9e096e 100644 --- a/src/biodata_schema/components/subject_procedures.py +++ b/src/biodata_schema/components/subject_procedures.py @@ -8,7 +8,7 @@ from pydantic import Field from biodata_schema.base import DataModel, DiscriminatedList -from biodata_schema.components.coordinates import CoordinateSystemOrNotApplicable, Translation +from biodata_schema.components.coordinates import CoordinateSystemOrNA, Translation from biodata_schema.components.identifiers import Code, ProtocolMixin from biodata_schema.components.injection_procedures import Injection from biodata_schema.components.surgery_procedures import ( @@ -122,7 +122,7 @@ class Surgery(ProtocolMixin, DataModel): workstation_id: Optional[str] = Field(default=None, title="Workstation ID") # Coordinate system - global_coordinate_system: Optional[CoordinateSystemOrNotApplicable] = Field( + global_coordinate_system: Optional[CoordinateSystemOrNA] = Field( default=None, title="Surgery global coordinate system", description=( diff --git a/src/biodata_schema/core/acquisition.py b/src/biodata_schema/core/acquisition.py index d7699a7e1..d28e3df60 100644 --- a/src/biodata_schema/core/acquisition.py +++ b/src/biodata_schema/core/acquisition.py @@ -43,7 +43,7 @@ SpeakerConfig, ) from biodata_schema.components.connections import Connection -from biodata_schema.components.coordinates import CoordinateSystemOrNotApplicable +from biodata_schema.components.coordinates import CoordinateSystemOrNA from biodata_schema.components.identifiers import Code, ProtocolListMixin from biodata_schema.components.measurements import CALIBRATIONS, Maintenance from biodata_schema.components.reagent import Reagent @@ -448,7 +448,7 @@ def reject_non_iana_tz(cls, v): notes: Optional[str] = Field(default=None, title="Notes") # Coordinate system - global_coordinate_system: Optional[CoordinateSystemOrNotApplicable] = Field( + global_coordinate_system: Optional[CoordinateSystemOrNA] = Field( default=None, title="Global coordinate system", description=( diff --git a/src/biodata_schema/core/instrument.py b/src/biodata_schema/core/instrument.py index f953c8c02..3604fe9ca 100644 --- a/src/biodata_schema/core/instrument.py +++ b/src/biodata_schema/core/instrument.py @@ -9,7 +9,7 @@ from biodata_schema.base import DataCoreModel, DiscriminatedList, DraftRequirement from biodata_schema.components.connections import Connection -from biodata_schema.components.coordinates import CoordinateSystemOrNotApplicable +from biodata_schema.components.coordinates import CoordinateSystemOrNA from biodata_schema.components.devices import ( AdditionalImagingDevice, AirPuffDevice, @@ -104,7 +104,7 @@ class Instrument(DataCoreModel): ) # coordinate system - global_coordinate_system: CoordinateSystemOrNotApplicable = Field( + global_coordinate_system: CoordinateSystemOrNA = Field( ..., title="Global coordinate system", description=( diff --git a/src/biodata_schema/core/procedures.py b/src/biodata_schema/core/procedures.py index 6aa2ba620..20b7e504c 100644 --- a/src/biodata_schema/core/procedures.py +++ b/src/biodata_schema/core/procedures.py @@ -5,7 +5,7 @@ from pydantic import Field, SkipValidation, model_validator from biodata_schema.base import DataCoreModel, DiscriminatedList -from biodata_schema.components.coordinates import CoordinateSystemOrNotApplicable +from biodata_schema.components.coordinates import CoordinateSystemOrNA from biodata_schema.components.specimen_procedures import SpecimenProcedure from biodata_schema.components.subject_procedures import ( GenericSubjectProcedure, @@ -38,7 +38,7 @@ class Procedures(DataCoreModel): ) # Coordinate system - global_coordinate_system: Optional[CoordinateSystemOrNotApplicable] = Field( + global_coordinate_system: Optional[CoordinateSystemOrNA] = Field( default=None, title="Global Coordinate System", description=( diff --git a/tests/test_coordinates.py b/tests/test_coordinates.py index dc1162414..0ca845520 100644 --- a/tests/test_coordinates.py +++ b/tests/test_coordinates.py @@ -10,7 +10,7 @@ Axis, AxisName, CoordinateSystem, - CoordinateSystemOrNotApplicable, + CoordinateSystemOrNA, Direction, Handedness, Origin, @@ -48,7 +48,7 @@ def test_not_applicable_is_not_a_coordinate_model_field(): """The class constant does not change real coordinate-system objects.""" assert "NotApplicable" not in CoordinateSystem.model_fields assert "NotApplicable" not in BREGMA_ARI.model_dump() - adapter = TypeAdapter(CoordinateSystemOrNotApplicable) + adapter = TypeAdapter(CoordinateSystemOrNA) assert adapter.validate_json(adapter.dump_json(BREGMA_ARI)) == BREGMA_ARI From 47fa004967b82180cd8c73b7b9a1892611b92110 Mon Sep 17 00:00:00 2001 From: Dan Birman Date: Fri, 9 Oct 2026 14:25:38 -0700 Subject: [PATCH 4/5] fix: allow merging CS with NA (CS overrides) --- src/biodata_schema/core/instrument.py | 14 +++++--- src/biodata_schema/utils/merge.py | 9 ++++- tests/test_coordinates.py | 48 ++++++++++++++++++++++++--- 3 files changed, 62 insertions(+), 9 deletions(-) diff --git a/src/biodata_schema/core/instrument.py b/src/biodata_schema/core/instrument.py index 3604fe9ca..e626f9091 100644 --- a/src/biodata_schema/core/instrument.py +++ b/src/biodata_schema/core/instrument.py @@ -55,7 +55,12 @@ Wheel, ) from biodata_schema.components.measurements import CALIBRATIONS -from biodata_schema.utils.merge import merge_notes, merge_optional_list, merge_str_alphabetical +from biodata_schema.utils.merge import ( + merge_coordinate_systems, + merge_notes, + merge_optional_list, + merge_str_alphabetical, +) from biodata_schema.utils.validators import recursive_get_device_names, recursive_get_named_objects logger = logging.getLogger(__name__) @@ -362,10 +367,9 @@ def __add__(self, other: "Instrument") -> "Instrument": # Check for incompatible key fields location_check = self.location != other.location - coord_sys_check = self.global_coordinate_system != other.global_coordinate_system temp_control_check = self.temperature_control != other.temperature_control - if any([location_check, coord_sys_check, temp_control_check]): + if any([location_check, temp_control_check]): raise ValueError( "Cannot combine Instrument objects that differ in key fields:\n" f"location: {self.location}/{other.location}\n" @@ -373,6 +377,8 @@ def __add__(self, other: "Instrument") -> "Instrument": f"temperature_control: {self.temperature_control}/{other.temperature_control}" ) + coordinate_system = merge_coordinate_systems(self.global_coordinate_system, other.global_coordinate_system) + # Combine instrument_name instrument_name = merge_str_alphabetical(self.instrument_name, other.instrument_name) @@ -401,7 +407,7 @@ def __add__(self, other: "Instrument") -> "Instrument": modification_date=latest_modification_date, modalities=combined_modalities, calibrations=combined_calibrations, - global_coordinate_system=self.global_coordinate_system, + global_coordinate_system=coordinate_system, temperature_control=self.temperature_control, notes=combined_notes, connections=combined_connections, diff --git a/src/biodata_schema/utils/merge.py b/src/biodata_schema/utils/merge.py index 9ae2f2d68..94a49a6b9 100644 --- a/src/biodata_schema/utils/merge.py +++ b/src/biodata_schema/utils/merge.py @@ -108,7 +108,14 @@ def merge_notes(notes1: Optional[str], notes2: Optional[str]) -> Optional[str]: def merge_coordinate_systems(cs1: Optional[Any], cs2: Optional[Any]) -> Optional[Any]: - """Merge coordinate systems, including explicit not-applicable values.""" + """Merge coordinate systems, preferring a real frame over not-applicable values.""" + + from biodata_schema.components.coordinates import CoordinateSystem + + if cs1 == CoordinateSystem.NotApplicable and isinstance(cs2, CoordinateSystem): + return cs2 + if cs2 == CoordinateSystem.NotApplicable and isinstance(cs1, CoordinateSystem): + return cs1 if cs1 and cs2: if cs1 != cs2: diff --git a/tests/test_coordinates.py b/tests/test_coordinates.py index 0ca845520..4ee353b6c 100644 --- a/tests/test_coordinates.py +++ b/tests/test_coordinates.py @@ -68,10 +68,50 @@ def test_merge_not_applicable_coordinate_systems(first, second, expected): @pytest.mark.parametrize( "first,second", [(CoordinateSystem.NotApplicable, BREGMA_ARI), (BREGMA_ARI, CoordinateSystem.NotApplicable)] ) -def test_merge_not_applicable_with_real_frame_fails(first, second): - """Contradictory frames raise a validation error, not an attribute error.""" - with pytest.raises(ValueError, match="Cannot merge differing coordinate systems"): - merge_coordinate_systems(first, second) +def test_merge_not_applicable_with_real_frame(first, second): + """A real frame overrides not-applicable in either operand order.""" + assert merge_coordinate_systems(first, second) == BREGMA_ARI + + +@pytest.mark.parametrize("model", [Instrument, Acquisition, Procedures]) +@pytest.mark.parametrize("real_first", [True, False]) +def test_core_merge_real_frame_overrides_not_applicable(model, real_first): + """Core-model addition keeps the real frame and validates in both operand orders.""" + if model is Instrument: + from examples.ephys_instrument import inst as spatial + + nonspatial = Instrument( + instrument_name=spatial.instrument_name, + modification_date=spatial.modification_date, + location=spatial.location, + temperature_control=spatial.temperature_control, + modalities=[], + components=[], + global_coordinate_system=CoordinateSystem.NotApplicable, + ) + elif model is Acquisition: + from examples.ephys_acquisition import acquisition as spatial + + nonspatial = Acquisition( + subject_name=spatial.subject_name, + instrument_name=spatial.instrument_name, + acquisition_start_time=spatial.acquisition_start_time, + acquisition_end_time=spatial.acquisition_end_time, + acquisition_type=spatial.acquisition_type, + data_streams=[], + global_coordinate_system=CoordinateSystem.NotApplicable, + ) + else: + from examples.thermistor_procedures import p as spatial + + nonspatial = Procedures( + subject_name=spatial.subject_name, + global_coordinate_system=CoordinateSystem.NotApplicable, + ) + combined = spatial + nonspatial if real_first else nonspatial + spatial + assert combined.global_coordinate_system == spatial.global_coordinate_system + revalidated = model.model_validate_json(combined.model_dump_json()) + assert revalidated.global_coordinate_system == spatial.global_coordinate_system class TestTranslationFrame: From 63be96a63caaea1501f0e069c5f650106edfb18e Mon Sep 17 00:00:00 2001 From: Dan Birman Date: Fri, 9 Oct 2026 14:25:44 -0700 Subject: [PATCH 5/5] tests: unnecessary duplicate tests --- tests/test_examples.py | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/tests/test_examples.py b/tests/test_examples.py index ee9a1723d..8623dd0de 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -21,31 +21,6 @@ def generated_examples(tmp_path_factory): class TestExamples: """tests for examples""" - @pytest.mark.parametrize( - "example_name", - [ - "aibs_smartspim_instrument", - "aind_smartspim_instrument", - "barseq_instrument", - "exaspim_instrument", - "fip_behavior_instrument", - "fip_ophys_instrument", - "isi_instrument", - "slap2_instrument", - "barseq_acquisition", - "ophys_acquisition", - "fip_ophys_acquisition", - "aibs_smartspim_procedures", - "ophys_procedures", - "procedures", - ], - ) - def test_position_free_global_frames_are_not_applicable(self, generated_examples, example_name): - """Examples without positions in their global scope explicitly mark it not applicable.""" - with (generated_examples / f"{example_name}.json").open() as example_file: - data = json.load(example_file) - assert data["global_coordinate_system"] == "Not applicable" - def test_examples_generated(self, generated_examples): """Test that each example file generates valid JSON.""" example_files = [