Merge pull request #1122 from AllenNeuralDynamics/1091-evaluate-diffi…

…cult-of-converting-units-to-_unit-with-global-validator Refactor _unit fields to rely on a validator instead of always being set
AllenNeuralDynamics · Dec 12, 2024 · 7ee0989 · 7ee0989
2 parents 89063e0 + a2c4f1a
commit 7ee0989
Show file tree

Hide file tree

Showing 35 changed files with 301 additions and 132 deletions.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -80,10 +80,29 @@ There are several libraries used to run linters and check documentation. We've i
   ```
 **NOTE**: Please note that these linters are automatically run in github actions when a PR is opened. These linters must pass for a PR to merge. 
 
+### Units
+Unit types (i.e. anything from [aind_data_schema_models.units](https://github.com/AllenNeuralDynamics/aind-data-schema-models/blob/main/src/aind_data_schema_models/units.py)) should always be paired with a variable in one of two patterns.
+
+When you have a single `variable` with a unit, you should add the `_unit` suffix on the name of the unit:
+
+```python
+variable: type = Field(...)
+variable_unit: XUnit = Field(...)
+```
+
+If the variable is `Optional[]` the unit should also be marked as optional.
+
+If you have multiple variables that map onto a single unit type, start each `variable` with the same prefix. The prefix should be unique within the class (watch out for inherited fields). 
+
+```python
+fov_width: type = Field(...)
+fov_height: type = Field(...)
+fov_unit: XUnit = Field(...)
+```
+
 ## Documentation and Style Guide
 Documentation is required for contributing to this project. We have settled on using Numpy's conventions as a default: [Numpy docstring standards](https://numpydoc.readthedocs.io/en/latest/format.html)
 
-
 ## Pull Requests
 For internal members, please create a branch. For external members, please fork the repo and open a pull request from the fork. We'll primarily use [Angular](https://github.com/angular/angular/blob/main/CONTRIBUTING.md#commit) style for commit messages. Roughly, they should follow the pattern:
 ```

diff --git a/examples/aibs_smartspim_instrument.json b/examples/aibs_smartspim_instrument.json
@@ -84,7 +84,7 @@
          "cooling": "Air",
          "computer_name": null,
          "frame_rate": null,
-         "frame_rate_unit": "hertz",
+         "frame_rate_unit": null,
          "immersion": null,
          "chroma": null,
          "sensor_width": null,

diff --git a/examples/aibs_smartspim_instrument.py b/examples/aibs_smartspim_instrument.py
@@ -3,6 +3,7 @@
 import datetime
 
 from aind_data_schema_models.organizations import Organization
+from aind_data_schema_models.units import SizeUnit
 
 from aind_data_schema.components.devices import (
     AdditionalImagingDevice,
@@ -166,6 +167,7 @@
             manufacturer=Organization.SEMROCK,
             diameter=25,
             thickness=2.0,
+            thickness_unit=SizeUnit.MM,
             model="FF03-525/50-25",
             filter_wheel_index=0,
         ),
@@ -175,6 +177,7 @@
             manufacturer=Organization.SEMROCK,
             diameter=25,
             thickness=2.0,
+            thickness_unit=SizeUnit.MM,
             model="FF01-600/52-25",
             filter_wheel_index=1,
         ),
@@ -184,6 +187,7 @@
             manufacturer=Organization.CHROMA,
             diameter=25,
             thickness=2.0,
+            thickness_unit=SizeUnit.MM,
             model="ET690/50m",
             filter_wheel_index=2,
         ),

diff --git a/examples/aind_smartspim_instrument.json b/examples/aind_smartspim_instrument.json
@@ -104,7 +104,7 @@
          "cooling": "Air",
          "computer_name": null,
          "frame_rate": null,
-         "frame_rate_unit": "hertz",
+         "frame_rate_unit": null,
          "immersion": null,
          "chroma": null,
          "sensor_width": null,

diff --git a/examples/aind_smartspim_instrument.py b/examples/aind_smartspim_instrument.py
@@ -3,6 +3,7 @@
 import datetime
 
 from aind_data_schema_models.organizations import Organization
+from aind_data_schema_models.units import SizeUnit
 
 from aind_data_schema.components.devices import Filter, Laser, MotorizedStage, OpticalTable, ScanningStage
 from aind_data_schema.core.instrument import Com, Detector, Instrument, Objective
@@ -106,6 +107,7 @@
             manufacturer=Organization.SEMROCK,
             diameter=25,
             thickness=2.0,
+            thickness_unit=SizeUnit.MM,
             model="FF01-469/35-25",
             filter_wheel_index=0,
             serial_number="Unknown-0",
@@ -116,6 +118,7 @@
             manufacturer=Organization.SEMROCK,
             diameter=25,
             thickness=2.0,
+            thickness_unit=SizeUnit.MM,
             model="FF01-525/45-25",
             filter_wheel_index=1,
             serial_number="Unknown-1",
@@ -126,6 +129,7 @@
             manufacturer=Organization.SEMROCK,
             diameter=25,
             thickness=2.0,
+            thickness_unit=SizeUnit.MM,
             model="FF01-593/40-25",
             filter_wheel_index=2,
             serial_number="Unknown-2",
@@ -136,6 +140,7 @@
             manufacturer=Organization.SEMROCK,
             diameter=25,
             thickness=2.0,
+            thickness_unit=SizeUnit.MM,
             model="FF01-624/40-25",
             filter_wheel_index=3,
             serial_number="Unknown-3",
@@ -146,6 +151,7 @@
             manufacturer=Organization.CHROMA,
             diameter=25,
             thickness=2.0,
+            thickness_unit=SizeUnit.MM,
             model="ET667/30m",
             filter_wheel_index=4,
             serial_number="Unknown-4",
@@ -156,6 +162,7 @@
             manufacturer=Organization.THORLABS,
             diameter=25,
             thickness=2.0,
+            thickness_unit=SizeUnit.MM,
             model="FELH0700",
             filter_wheel_index=5,
             serial_number="Unknown-5",

diff --git a/examples/bergamo_ophys_session.json b/examples/bergamo_ophys_session.json
@@ -68,10 +68,10 @@
                "frame_rate_unit": "hertz",
                "coupled_fov_index": null,
                "power": null,
-               "power_unit": "percent",
+               "power_unit": null,
                "power_ratio": null,
                "scanfield_z": null,
-               "scanfield_z_unit": "micrometer",
+               "scanfield_z_unit": null,
                "scanimage_roi_index": null,
                "notes": null
             }

diff --git a/examples/bergamo_ophys_session.py b/examples/bergamo_ophys_session.py
@@ -3,6 +3,7 @@
 from datetime import datetime, timezone
 
 from aind_data_schema_models.modalities import Modality
+from aind_data_schema_models.units import FrequencyUnit
 
 from aind_data_schema.components.stimulus import PhotoStimulation, PhotoStimulationGroup
 from aind_data_schema.core.session import (
@@ -64,6 +65,7 @@
                     magnification="1x",
                     fov_scale_factor=1.5,
                     frame_rate=20,
+                    frame_rate_unit=FrequencyUnit.HZ,
                 ),
             ],
         ),

diff --git a/examples/ephys_rig.json b/examples/ephys_rig.json
@@ -123,7 +123,7 @@
             "height": null,
             "size_unit": "millimeter",
             "thickness": null,
-            "thickness_unit": "millimeter",
+            "thickness_unit": null,
             "filter_wheel_index": null,
             "cut_off_wavelength": null,
             "cut_on_wavelength": null,
@@ -232,7 +232,7 @@
             "height": null,
             "size_unit": "millimeter",
             "thickness": null,
-            "thickness_unit": "millimeter",
+            "thickness_unit": null,
             "filter_wheel_index": null,
             "cut_off_wavelength": null,
             "cut_on_wavelength": null,
@@ -386,7 +386,7 @@
             "additional_settings": {},
             "notes": null,
             "focal_length": null,
-            "focal_length_unit": "millimeter",
+            "focal_length_unit": null,
             "size": null,
             "lens_size_unit": "inch",
             "optimized_wavelength_range": null,
@@ -464,7 +464,7 @@
             "additional_settings": {},
             "notes": null,
             "focal_length": null,
-            "focal_length_unit": "millimeter",
+            "focal_length_unit": null,
             "size": null,
             "lens_size_unit": "inch",
             "optimized_wavelength_range": null,
@@ -542,7 +542,7 @@
             "additional_settings": {},
             "notes": null,
             "focal_length": null,
-            "focal_length_unit": "millimeter",
+            "focal_length_unit": null,
             "size": null,
             "lens_size_unit": "inch",
             "optimized_wavelength_range": null,
@@ -620,7 +620,7 @@
             "additional_settings": {},
             "notes": null,
             "focal_length": null,
-            "focal_length_unit": "millimeter",
+            "focal_length_unit": null,
             "size": null,
             "lens_size_unit": "inch",
             "optimized_wavelength_range": null,
@@ -815,7 +815,7 @@
                "port": null,
                "channel_index": null,
                "sample_rate": null,
-               "sample_rate_unit": "hertz",
+               "sample_rate_unit": null,
                "event_based_sampling": null
             },
             {
@@ -825,7 +825,7 @@
                "port": null,
                "channel_index": null,
                "sample_rate": null,
-               "sample_rate_unit": "hertz",
+               "sample_rate_unit": null,
                "event_based_sampling": null
             },
             {
@@ -835,7 +835,7 @@
                "port": null,
                "channel_index": null,
                "sample_rate": null,
-               "sample_rate_unit": "hertz",
+               "sample_rate_unit": null,
                "event_based_sampling": null
             }
          ],

diff --git a/examples/ephys_rig.py b/examples/ephys_rig.py
@@ -5,6 +5,7 @@
 from aind_data_schema_models.harp_types import HarpDeviceType
 from aind_data_schema_models.modalities import Modality
 from aind_data_schema_models.organizations import Organization
+from aind_data_schema_models.units import FrequencyUnit, SizeUnit
 
 from aind_data_schema.components.devices import (
     Calibration,
@@ -91,6 +92,7 @@
     manufacturer=Organization.FLIR,
     computer_name=ephys_computer,
     frame_rate=50,
+    frame_rate_unit=FrequencyUnit.HZ,
     sensor_width=1080,
     sensor_height=570,
     sensor_format="1/2.9",
@@ -105,6 +107,7 @@
     manufacturer=Organization.FLIR,
     computer_name=ephys_computer,
     frame_rate=50,
+    frame_rate_unit=FrequencyUnit.HZ,
     sensor_width=1080,
     sensor_height=570,
     sensor_format="1/2.9",
@@ -119,6 +122,7 @@
     manufacturer=Organization.FLIR,
     computer_name=ephys_computer,
     frame_rate=50,
+    frame_rate_unit=FrequencyUnit.HZ,
     sensor_width=1080,
     sensor_height=570,
     sensor_format="1/2.9",
@@ -133,6 +137,7 @@
     manufacturer=Organization.FLIR,
     computer_name=ephys_computer,
     frame_rate=50,
+    frame_rate_unit=FrequencyUnit.HZ,
     sensor_width=1080,
     sensor_height=570,
     sensor_format="1/2.9",
@@ -197,7 +202,13 @@
     description="850 nm longpass filter",
 )
 
-lens = Lens(name="Camera lens", focal_length=15, manufacturer=Organization.EDMUND_OPTICS, max_aperture="f/2")
+lens = Lens(
+    name="Camera lens",
+    focal_length=15,
+    focal_length_unit=SizeUnit.MM,
+    manufacturer=Organization.EDMUND_OPTICS,
+    max_aperture="f/2",
+)
 
 face_camera = Camera(
     name="Face Camera",
@@ -206,6 +217,7 @@
     manufacturer=Organization.FLIR,
     computer_name=behavior_computer,
     frame_rate=50,
+    frame_rate_unit=FrequencyUnit.HZ,
     sensor_width=1080,
     sensor_height=570,
     sensor_format="1/2.9",
@@ -228,6 +240,7 @@
     manufacturer=Organization.FLIR,
     computer_name=behavior_computer,
     frame_rate=50,
+    frame_rate_unit=FrequencyUnit.HZ,
     sensor_width=1080,
     sensor_height=570,
     sensor_format="1/2.9",

diff --git a/examples/ephys_session.json b/examples/ephys_session.json
@@ -57,7 +57,7 @@
                "anatomical_coordinates": null,
                "anatomical_reference": null,
                "surface_z": null,
-               "surface_z_unit": "micrometer",
+               "surface_z_unit": null,
                "dye": null,
                "implant_hole_number": null
             },
@@ -90,7 +90,7 @@
                "anatomical_coordinates": null,
                "anatomical_reference": null,
                "surface_z": null,
-               "surface_z_unit": "micrometer",
+               "surface_z_unit": null,
                "dye": null,
                "implant_hole_number": null
             }
@@ -192,7 +192,7 @@
                "anatomical_coordinates": null,
                "anatomical_reference": null,
                "surface_z": null,
-               "surface_z_unit": "micrometer",
+               "surface_z_unit": null,
                "dye": null,
                "implant_hole_number": null
             },
@@ -225,7 +225,7 @@
                "anatomical_coordinates": null,
                "anatomical_reference": null,
                "surface_z": null,
-               "surface_z_unit": "micrometer",
+               "surface_z_unit": null,
                "dye": null,
                "implant_hole_number": null
             }

diff --git a/examples/exaspim_acquisition.json b/examples/exaspim_acquisition.json
@@ -97,7 +97,7 @@
             "excitation_power_unit": "milliwatt",
             "filter_wheel_index": 0,
             "dilation": null,
-            "dilation_unit": "pixel",
+            "dilation_unit": null,
             "description": null
          },
          "notes": "these are my notes",
@@ -140,7 +140,7 @@
             "excitation_power_unit": "milliwatt",
             "filter_wheel_index": 0,
             "dilation": null,
-            "dilation_unit": "pixel",
+            "dilation_unit": null,
             "description": null
          },
          "notes": "these are my notes",

diff --git a/examples/exaspim_instrument.json b/examples/exaspim_instrument.json
@@ -81,7 +81,7 @@
          "cooling": "Air",
          "computer_name": null,
          "frame_rate": null,
-         "frame_rate_unit": "hertz",
+         "frame_rate_unit": null,
          "immersion": null,
          "chroma": null,
          "sensor_width": null,