From 25f96e32102cd2a3fa10edeaecb37472e4cbf0af Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Tue, 24 Mar 2026 13:27:41 -0600 Subject: [PATCH 1/7] Add spatial series units check --- CHANGELOG.md | 1 + docs/best_practices/behavior.rst | 12 +++++++ src/nwbinspector/checks/__init__.py | 2 ++ src/nwbinspector/checks/_behavior.py | 35 +++++++++++++++++++ tests/unit_tests/test_behavior.py | 52 ++++++++++++++++++++++++++++ 5 files changed, 102 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5574f305f..06c417b86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ### New Checks +* Added `check_spatial_series_unit` to validate that SpatialSeries objects (outside of CompassDirection) have a recognized unit string from a curated allowlist of SI length units, angular units, and pixels. [#685](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/685) * Added `check_imaging_plane_location_allen_ccf`, `check_electrodes_location_allen_ccf`, and `check_intracellular_electrode_location_allen_ccf` to validate location fields against Allen Mouse Brain CCF ontology terms when subject species is mouse. [#671](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/671) diff --git a/docs/best_practices/behavior.rst b/docs/best_practices/behavior.rst index df68e6968..147a279a1 100644 --- a/docs/best_practices/behavior.rst +++ b/docs/best_practices/behavior.rst @@ -28,6 +28,18 @@ When a :ref:`nwb-schema:sec-SpatialSeries` is in a :ref:`nwb-schema:sec-CompassD Check function: :py:meth:`~nwbinspector.checks._behavior.check_compass_direction_unit` +.. _best_practice_spatial_series_general_units: + +SpatialSeries General Units +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +A :ref:`nwb-schema:sec-SpatialSeries` that is not inside a :ref:`nwb-schema:sec-CompassDirection` should have a unit from +the following recognized set: "meters", "centimeters", "millimeters", "micrometers", "degrees", "radians", or "pixels". +These follow the PyNWB convention of full SI words in plural form. + +Check function: :py:meth:`~nwbinspector.checks._behavior.check_spatial_series_unit` + + .. _best_practice_spatial_series_values: SpatialSeries Data Values diff --git a/src/nwbinspector/checks/__init__.py b/src/nwbinspector/checks/__init__.py index 266dbc336..283741884 100644 --- a/src/nwbinspector/checks/__init__.py +++ b/src/nwbinspector/checks/__init__.py @@ -3,6 +3,7 @@ check_spatial_series_degrees_magnitude, check_spatial_series_dims, check_spatial_series_radians_magnitude, + check_spatial_series_unit, ) from ._ecephys import ( check_ascending_spike_times, @@ -176,6 +177,7 @@ "check_spatial_series_radians_magnitude", "check_spatial_series_dims", "check_spatial_series_degrees_magnitude", + "check_spatial_series_unit", "check_ascending_spike_times", "check_electrical_series_unscaled_data", "check_rate_is_positive", diff --git a/src/nwbinspector/checks/_behavior.py b/src/nwbinspector/checks/_behavior.py index bca5af861..88683d3ca 100644 --- a/src/nwbinspector/checks/_behavior.py +++ b/src/nwbinspector/checks/_behavior.py @@ -76,3 +76,38 @@ def check_spatial_series_degrees_magnitude( ) return None + + +VALID_SPATIAL_SERIES_UNITS = { + "meters", + "centimeters", + "millimeters", + "micrometers", + "degrees", + "radians", + "pixels", +} + + +@register_check(importance=Importance.BEST_PRACTICE_VIOLATION, neurodata_type=SpatialSeries) +def check_spatial_series_unit(spatial_series: SpatialSeries) -> Optional[InspectorMessage]: + """ + Check that SpatialSeries.unit is a recognized spatial unit. + + Best Practice: :ref:`best_practice_spatial_series_general_units` + """ + if spatial_series.unit is None: + return None + + if spatial_series.get_ancestor("CompassDirection") is not None: + return None + + if spatial_series.unit not in VALID_SPATIAL_SERIES_UNITS: + return InspectorMessage( + message=( + f"SpatialSeries unit '{spatial_series.unit}' is not recognized. " + f"Valid units are: {', '.join(sorted(VALID_SPATIAL_SERIES_UNITS))}." + ) + ) + + return None diff --git a/tests/unit_tests/test_behavior.py b/tests/unit_tests/test_behavior.py index ee40e6194..331e6720d 100644 --- a/tests/unit_tests/test_behavior.py +++ b/tests/unit_tests/test_behavior.py @@ -7,6 +7,7 @@ check_spatial_series_degrees_magnitude, check_spatial_series_dims, check_spatial_series_radians_magnitude, + check_spatial_series_unit, ) @@ -149,3 +150,54 @@ def test_check_spatial_series_radians_magnitude(): location="/", object_type="SpatialSeries", ) + + +def test_pass_check_spatial_series_unit(): + for unit in ("meters", "centimeters", "millimeters", "micrometers", "degrees", "radians", "pixels"): + spatial_series = SpatialSeries( + name="SpatialSeries", + description="description", + data=np.ones((10,)), + rate=3.0, + reference_frame="reference_frame", + unit=unit, + ) + assert check_spatial_series_unit(spatial_series) is None + + +def test_fail_check_spatial_series_unit(): + spatial_series = SpatialSeries( + name="SpatialSeries", + description="description", + data=np.ones((10,)), + rate=3.0, + reference_frame="reference_frame", + unit="kilometers", + ) + result = check_spatial_series_unit(spatial_series) + assert result == InspectorMessage( + message=( + "SpatialSeries unit 'kilometers' is not recognized. " + "Valid units are: centimeters, degrees, meters, micrometers, millimeters, pixels, radians." + ), + importance=Importance.BEST_PRACTICE_VIOLATION, + check_function_name="check_spatial_series_unit", + object_type="SpatialSeries", + object_name="SpatialSeries", + location="/", + ) + + +def test_skip_check_spatial_series_unit_in_compass_direction(): + compass_direction = CompassDirection( + spatial_series=SpatialSeries( + name="SpatialSeries", + description="description", + data=np.ones((10,)), + rate=3.0, + reference_frame="reference_frame", + unit="degrees", + ) + ) + spatial_series = compass_direction.get_spatial_series("SpatialSeries") + assert check_spatial_series_unit(spatial_series) is None From daf539bb48828856f7c1e22a0145c00bb0fa6d8b Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Tue, 24 Mar 2026 16:50:23 -0600 Subject: [PATCH 2/7] Improve testing --- src/nwbinspector/checks/_behavior.py | 2 +- tests/unit_tests/test_behavior.py | 15 +-------------- 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/src/nwbinspector/checks/_behavior.py b/src/nwbinspector/checks/_behavior.py index 88683d3ca..a3e33d2ef 100644 --- a/src/nwbinspector/checks/_behavior.py +++ b/src/nwbinspector/checks/_behavior.py @@ -105,7 +105,7 @@ def check_spatial_series_unit(spatial_series: SpatialSeries) -> Optional[Inspect if spatial_series.unit not in VALID_SPATIAL_SERIES_UNITS: return InspectorMessage( message=( - f"SpatialSeries unit '{spatial_series.unit}' is not recognized. " + f"SpatialSeries unit '{spatial_series.unit}' is not a valid spatial unit. " f"Valid units are: {', '.join(sorted(VALID_SPATIAL_SERIES_UNITS))}." ) ) diff --git a/tests/unit_tests/test_behavior.py b/tests/unit_tests/test_behavior.py index 331e6720d..c0a2ed3eb 100644 --- a/tests/unit_tests/test_behavior.py +++ b/tests/unit_tests/test_behavior.py @@ -177,7 +177,7 @@ def test_fail_check_spatial_series_unit(): result = check_spatial_series_unit(spatial_series) assert result == InspectorMessage( message=( - "SpatialSeries unit 'kilometers' is not recognized. " + "SpatialSeries unit 'kilometers' is not a valid spatial unit. " "Valid units are: centimeters, degrees, meters, micrometers, millimeters, pixels, radians." ), importance=Importance.BEST_PRACTICE_VIOLATION, @@ -188,16 +188,3 @@ def test_fail_check_spatial_series_unit(): ) -def test_skip_check_spatial_series_unit_in_compass_direction(): - compass_direction = CompassDirection( - spatial_series=SpatialSeries( - name="SpatialSeries", - description="description", - data=np.ones((10,)), - rate=3.0, - reference_frame="reference_frame", - unit="degrees", - ) - ) - spatial_series = compass_direction.get_spatial_series("SpatialSeries") - assert check_spatial_series_unit(spatial_series) is None From ac7259b02eb052e170a4880be3e602c03d34931b Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 24 Mar 2026 22:50:37 +0000 Subject: [PATCH 3/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/unit_tests/test_behavior.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/unit_tests/test_behavior.py b/tests/unit_tests/test_behavior.py index c0a2ed3eb..017eda4a6 100644 --- a/tests/unit_tests/test_behavior.py +++ b/tests/unit_tests/test_behavior.py @@ -186,5 +186,3 @@ def test_fail_check_spatial_series_unit(): object_name="SpatialSeries", location="/", ) - - From 39e6ab85b8867ac1f392e4ea29c408ef9873df41 Mon Sep 17 00:00:00 2001 From: Ben Dichter Date: Wed, 25 Mar 2026 09:13:12 -0400 Subject: [PATCH 4/7] Use invalid unit in CompassDirection skip test Use 'kilometers' instead of 'degrees' so the test actually exercises the CompassDirection ancestor check rather than passing due to a valid unit. Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/unit_tests/test_behavior.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit_tests/test_behavior.py b/tests/unit_tests/test_behavior.py index a723d9433..34e902554 100644 --- a/tests/unit_tests/test_behavior.py +++ b/tests/unit_tests/test_behavior.py @@ -185,7 +185,7 @@ def test_skip_check_spatial_series_unit_in_compass_direction(): data=np.ones((10,)), rate=3.0, reference_frame="reference_frame", - unit="degrees", + unit="kilometers", ) ) spatial_series = compass_direction.spatial_series["SpatialSeries"] From 8adb440970e130702907a8844ec32efe5d714673 Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Thu, 26 Mar 2026 11:04:45 -0600 Subject: [PATCH 5/7] add .n.a. placeholder --- docs/best_practices/behavior.rst | 3 ++- src/nwbinspector/checks/_behavior.py | 4 +++- tests/unit_tests/test_behavior.py | 5 +++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/docs/best_practices/behavior.rst b/docs/best_practices/behavior.rst index 147a279a1..b422d5471 100644 --- a/docs/best_practices/behavior.rst +++ b/docs/best_practices/behavior.rst @@ -34,7 +34,8 @@ SpatialSeries General Units ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ A :ref:`nwb-schema:sec-SpatialSeries` that is not inside a :ref:`nwb-schema:sec-CompassDirection` should have a unit from -the following recognized set: "meters", "centimeters", "millimeters", "micrometers", "degrees", "radians", or "pixels". +the following recognized set: "meters", "centimeters", "millimeters", "micrometers", "degrees", "radians", "pixels", +or "n.a." (not available) as a placeholder when the unit is not known. These follow the PyNWB convention of full SI words in plural form. Check function: :py:meth:`~nwbinspector.checks._behavior.check_spatial_series_unit` diff --git a/src/nwbinspector/checks/_behavior.py b/src/nwbinspector/checks/_behavior.py index a3e33d2ef..00dc7ee73 100644 --- a/src/nwbinspector/checks/_behavior.py +++ b/src/nwbinspector/checks/_behavior.py @@ -86,6 +86,7 @@ def check_spatial_series_degrees_magnitude( "degrees", "radians", "pixels", + "n.a.", } @@ -106,7 +107,8 @@ def check_spatial_series_unit(spatial_series: SpatialSeries) -> Optional[Inspect return InspectorMessage( message=( f"SpatialSeries unit '{spatial_series.unit}' is not a valid spatial unit. " - f"Valid units are: {', '.join(sorted(VALID_SPATIAL_SERIES_UNITS))}." + f"Valid units are: {', '.join(sorted(VALID_SPATIAL_SERIES_UNITS))}. " + "If the unit is not known, use 'n.a.' (not available) as a placeholder." ) ) diff --git a/tests/unit_tests/test_behavior.py b/tests/unit_tests/test_behavior.py index 34e902554..b1d6c0030 100644 --- a/tests/unit_tests/test_behavior.py +++ b/tests/unit_tests/test_behavior.py @@ -153,7 +153,7 @@ def test_check_spatial_series_radians_magnitude(): def test_pass_check_spatial_series_unit(): - for unit in ("meters", "centimeters", "millimeters", "micrometers", "degrees", "radians", "pixels"): + for unit in ("meters", "centimeters", "millimeters", "micrometers", "degrees", "radians", "pixels", "n.a."): spatial_series = SpatialSeries( name="SpatialSeries", description="description", @@ -205,7 +205,8 @@ def test_fail_check_spatial_series_unit(): assert result == InspectorMessage( message=( "SpatialSeries unit 'kilometers' is not a valid spatial unit. " - "Valid units are: centimeters, degrees, meters, micrometers, millimeters, pixels, radians." + "Valid units are: centimeters, degrees, meters, micrometers, millimeters, n.a., pixels, radians. " + "If the unit is not known, use 'n.a.' (not available) as a placeholder." ), importance=Importance.BEST_PRACTICE_VIOLATION, check_function_name="check_spatial_series_unit", From 23ea24df256f8a752c3dc6283f71ddb379a3b1ca Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Thu, 26 Mar 2026 11:16:12 -0600 Subject: [PATCH 6/7] do not accept None --- src/nwbinspector/checks/_behavior.py | 3 --- tests/unit_tests/test_behavior.py | 12 ------------ 2 files changed, 15 deletions(-) diff --git a/src/nwbinspector/checks/_behavior.py b/src/nwbinspector/checks/_behavior.py index 00dc7ee73..cb8ee3a2b 100644 --- a/src/nwbinspector/checks/_behavior.py +++ b/src/nwbinspector/checks/_behavior.py @@ -97,9 +97,6 @@ def check_spatial_series_unit(spatial_series: SpatialSeries) -> Optional[Inspect Best Practice: :ref:`best_practice_spatial_series_general_units` """ - if spatial_series.unit is None: - return None - if spatial_series.get_ancestor("CompassDirection") is not None: return None diff --git a/tests/unit_tests/test_behavior.py b/tests/unit_tests/test_behavior.py index b1d6c0030..5ecb4094e 100644 --- a/tests/unit_tests/test_behavior.py +++ b/tests/unit_tests/test_behavior.py @@ -165,18 +165,6 @@ def test_pass_check_spatial_series_unit(): assert check_spatial_series_unit(spatial_series) is None -def test_skip_check_spatial_series_unit_none(): - spatial_series = SpatialSeries( - name="SpatialSeries", - description="description", - data=np.ones((10,)), - rate=3.0, - reference_frame="reference_frame", - ) - spatial_series.fields["unit"] = None - assert check_spatial_series_unit(spatial_series) is None - - def test_skip_check_spatial_series_unit_in_compass_direction(): compass_direction = CompassDirection( spatial_series=SpatialSeries( From 0860fdecd3a42cfcfa02bb8bfe1755ac423e6d56 Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Thu, 26 Mar 2026 11:17:07 -0600 Subject: [PATCH 7/7] Add test for None --- tests/unit_tests/test_behavior.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/unit_tests/test_behavior.py b/tests/unit_tests/test_behavior.py index 5ecb4094e..dd2154b22 100644 --- a/tests/unit_tests/test_behavior.py +++ b/tests/unit_tests/test_behavior.py @@ -202,3 +202,16 @@ def test_fail_check_spatial_series_unit(): object_name="SpatialSeries", location="/", ) + + +def test_fail_check_spatial_series_unit_none(): + spatial_series = SpatialSeries( + name="SpatialSeries", + description="description", + data=np.ones((10,)), + rate=3.0, + reference_frame="reference_frame", + ) + spatial_series.fields["unit"] = None + result = check_spatial_series_unit(spatial_series) + assert result is not None