From 2a13c39b2e14cfb30cce66aeab46ea678f2b15ce Mon Sep 17 00:00:00 2001 From: "jain.anoushka24" Date: Thu, 6 Mar 2025 13:40:40 +0100 Subject: [PATCH 1/9] Add check for negative sampling rate in time series --- src/nwbinspector/checks/__init__.py | 1 + src/nwbinspector/checks/_time_series.py | 12 ++++++++++++ tests/unit_tests/test_time_series.py | 15 +++++++++++++++ 3 files changed, 28 insertions(+) diff --git a/src/nwbinspector/checks/__init__.py b/src/nwbinspector/checks/__init__.py index a24e6323..5436898b 100644 --- a/src/nwbinspector/checks/__init__.py +++ b/src/nwbinspector/checks/__init__.py @@ -82,6 +82,7 @@ check_timestamps_ascending, check_timestamps_match_first_dimension, check_timestamps_without_nans, + check_rate_is_not_negative ) __all__ = [ diff --git a/src/nwbinspector/checks/_time_series.py b/src/nwbinspector/checks/_time_series.py index f6050fae..a28e66ba 100644 --- a/src/nwbinspector/checks/_time_series.py +++ b/src/nwbinspector/checks/_time_series.py @@ -180,5 +180,17 @@ def check_rate_is_not_zero(time_series: TimeSeries) -> Optional[InspectorMessage return InspectorMessage( f"{time_series.name} has a sampling rate value of 0.0Hz but the series has more than one frame." ) + + return None +@register_check(importance=Importance.CRITICAL, neurodata_type=TimeSeries) +def check_rate_is_not_negative(time_series: TimeSeries) -> Optional[InspectorMessage]: + if time_series.data is None: + return None + + if time_series.rate < 0.0 : + return InspectorMessage( + f"{time_series.name} has a negative sampling rate value of {time_series.rate}Hz.The sampling rate should have a positive value." + ) + return None diff --git a/tests/unit_tests/test_time_series.py b/tests/unit_tests/test_time_series.py index c1e6f309..63ec7828 100644 --- a/tests/unit_tests/test_time_series.py +++ b/tests/unit_tests/test_time_series.py @@ -13,6 +13,7 @@ check_timestamps_ascending, check_timestamps_match_first_dimension, check_timestamps_without_nans, + check_rate_is_not_negative ) from nwbinspector.testing import check_streaming_tests_enabled, make_minimal_nwbfile @@ -234,6 +235,20 @@ def test_check_rate_is_not_zero_fail(): location="/", ) +def test_check_rate_is_not_negative_pass(): + time_series = pynwb.TimeSeries(name="test", unit="test_units", data=[1, 2, 3], rate=4.0) + assert check_rate_is_not_negative(time_series) is None + +def test_check_rate_is_not_negative_fail(): + time_series = pynwb.TimeSeries(name="TimeSeriesTest", unit="n.a.", data=[1, 2, 3], rate=-4.0) + assert check_rate_is_not_negative(time_series) == InspectorMessage( + message= f"{time_series.name} has a negative sampling rate value of {time_series.rate}Hz.The sampling rate should have a positive value.", + importance=Importance.CRITICAL, + check_function_name="check_rate_is_not_negative", + object_type="TimeSeries", + object_name="TimeSeriesTest", + location="/", + ) def test_pass_check_timestamps_ascending_pass(): time_series = pynwb.TimeSeries(name="test_time_series", unit="test_units", data=[1, 2, 3], timestamps=[1, 2, 3]) From 8f26ee70deb14a7785261ef5f8b73343d438ae98 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 6 Mar 2025 12:43:17 +0000 Subject: [PATCH 2/9] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/nwbinspector/checks/__init__.py | 2 +- src/nwbinspector/checks/_time_series.py | 9 +++++---- tests/unit_tests/test_time_series.py | 9 ++++++--- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/nwbinspector/checks/__init__.py b/src/nwbinspector/checks/__init__.py index 5436898b..d1143cf3 100644 --- a/src/nwbinspector/checks/__init__.py +++ b/src/nwbinspector/checks/__init__.py @@ -75,6 +75,7 @@ from ._time_series import ( check_data_orientation, check_missing_unit, + check_rate_is_not_negative, check_rate_is_not_zero, check_regular_timestamps, check_resolution, @@ -82,7 +83,6 @@ check_timestamps_ascending, check_timestamps_match_first_dimension, check_timestamps_without_nans, - check_rate_is_not_negative ) __all__ = [ diff --git a/src/nwbinspector/checks/_time_series.py b/src/nwbinspector/checks/_time_series.py index a28e66ba..43739f56 100644 --- a/src/nwbinspector/checks/_time_series.py +++ b/src/nwbinspector/checks/_time_series.py @@ -180,17 +180,18 @@ def check_rate_is_not_zero(time_series: TimeSeries) -> Optional[InspectorMessage return InspectorMessage( f"{time_series.name} has a sampling rate value of 0.0Hz but the series has more than one frame." ) - + return None + @register_check(importance=Importance.CRITICAL, neurodata_type=TimeSeries) def check_rate_is_not_negative(time_series: TimeSeries) -> Optional[InspectorMessage]: if time_series.data is None: return None - - if time_series.rate < 0.0 : + + if time_series.rate < 0.0: return InspectorMessage( f"{time_series.name} has a negative sampling rate value of {time_series.rate}Hz.The sampling rate should have a positive value." ) - + return None diff --git a/tests/unit_tests/test_time_series.py b/tests/unit_tests/test_time_series.py index 63ec7828..54426224 100644 --- a/tests/unit_tests/test_time_series.py +++ b/tests/unit_tests/test_time_series.py @@ -6,6 +6,7 @@ from nwbinspector.checks import ( check_data_orientation, check_missing_unit, + check_rate_is_not_negative, check_rate_is_not_zero, check_regular_timestamps, check_resolution, @@ -13,7 +14,6 @@ check_timestamps_ascending, check_timestamps_match_first_dimension, check_timestamps_without_nans, - check_rate_is_not_negative ) from nwbinspector.testing import check_streaming_tests_enabled, make_minimal_nwbfile @@ -235,21 +235,24 @@ def test_check_rate_is_not_zero_fail(): location="/", ) + def test_check_rate_is_not_negative_pass(): time_series = pynwb.TimeSeries(name="test", unit="test_units", data=[1, 2, 3], rate=4.0) assert check_rate_is_not_negative(time_series) is None + def test_check_rate_is_not_negative_fail(): time_series = pynwb.TimeSeries(name="TimeSeriesTest", unit="n.a.", data=[1, 2, 3], rate=-4.0) assert check_rate_is_not_negative(time_series) == InspectorMessage( - message= f"{time_series.name} has a negative sampling rate value of {time_series.rate}Hz.The sampling rate should have a positive value.", + message=f"{time_series.name} has a negative sampling rate value of {time_series.rate}Hz.The sampling rate should have a positive value.", importance=Importance.CRITICAL, - check_function_name="check_rate_is_not_negative", + check_function_name="check_rate_is_not_negative", object_type="TimeSeries", object_name="TimeSeriesTest", location="/", ) + def test_pass_check_timestamps_ascending_pass(): time_series = pynwb.TimeSeries(name="test_time_series", unit="test_units", data=[1, 2, 3], timestamps=[1, 2, 3]) assert check_timestamps_ascending(time_series) is None From 75bd869b9ccd42134884bc4cf2d2d257572181c1 Mon Sep 17 00:00:00 2001 From: "jain.anoushka24" Date: Thu, 6 Mar 2025 14:05:34 +0100 Subject: [PATCH 3/9] Skip negative sampling rate test for pynwb versions >= 2.5.0 --- tests/unit_tests/test_time_series.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/unit_tests/test_time_series.py b/tests/unit_tests/test_time_series.py index 63ec7828..6b0dc08c 100644 --- a/tests/unit_tests/test_time_series.py +++ b/tests/unit_tests/test_time_series.py @@ -16,6 +16,8 @@ check_rate_is_not_negative ) from nwbinspector.testing import check_streaming_tests_enabled, make_minimal_nwbfile +import pytest +from packaging import version STREAMING_TESTS_ENABLED, DISABLED_STREAMING_TESTS_REASON = check_streaming_tests_enabled() @@ -240,7 +242,13 @@ def test_check_rate_is_not_negative_pass(): assert check_rate_is_not_negative(time_series) is None def test_check_rate_is_not_negative_fail(): + # TODO: Install pynwb < 2.5.0 to test this + # Check pynwb version and skip this test if pynwb >= 2.5.0 + if version.parse(pynwb.__version__) >= version.parse("2.5.0"): + pytest.skip("This test is not applicable for pynwb >= 2.5.0") + time_series = pynwb.TimeSeries(name="TimeSeriesTest", unit="n.a.", data=[1, 2, 3], rate=-4.0) + assert check_rate_is_not_negative(time_series) == InspectorMessage( message= f"{time_series.name} has a negative sampling rate value of {time_series.rate}Hz.The sampling rate should have a positive value.", importance=Importance.CRITICAL, From 4d882550c84af816aa6689eaa0e5173cf71bc690 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 6 Mar 2025 13:06:49 +0000 Subject: [PATCH 4/9] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/unit_tests/test_time_series.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit_tests/test_time_series.py b/tests/unit_tests/test_time_series.py index 60f85cb4..5fbb4934 100644 --- a/tests/unit_tests/test_time_series.py +++ b/tests/unit_tests/test_time_series.py @@ -1,6 +1,8 @@ import h5py import numpy as np import pynwb +import pytest +from packaging import version from nwbinspector import Importance, InspectorMessage from nwbinspector.checks import ( @@ -16,8 +18,6 @@ check_timestamps_without_nans, ) from nwbinspector.testing import check_streaming_tests_enabled, make_minimal_nwbfile -import pytest -from packaging import version STREAMING_TESTS_ENABLED, DISABLED_STREAMING_TESTS_REASON = check_streaming_tests_enabled() From 2167a003f5ac4be0c2d9d3dd15e957b98e069681 Mon Sep 17 00:00:00 2001 From: "jain.anoushka24" Date: Thu, 6 Mar 2025 14:16:54 +0100 Subject: [PATCH 5/9] Add check_rate_is_not_negative to public API --- src/nwbinspector/checks/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/nwbinspector/checks/__init__.py b/src/nwbinspector/checks/__init__.py index d1143cf3..b289aa6f 100644 --- a/src/nwbinspector/checks/__init__.py +++ b/src/nwbinspector/checks/__init__.py @@ -147,4 +147,5 @@ "check_spatial_series_radians_magnitude", "check_spatial_series_dims", "check_spatial_series_degrees_magnitude", + "check_rate_is_not_negative", ] From 66a722195a61dd70fa67307a6addc87219955ce1 Mon Sep 17 00:00:00 2001 From: "jain.anoushka24" Date: Thu, 6 Mar 2025 16:07:23 +0100 Subject: [PATCH 6/9] Add check_subject_weight_format to validate subject weight format --- src/nwbinspector/checks/__init__.py | 2 + src/nwbinspector/checks/_nwbfile_metadata.py | 18 +++++++++ tests/unit_tests/test_nwbfile_metadata.py | 41 ++++++++++++++++++++ 3 files changed, 61 insertions(+) diff --git a/src/nwbinspector/checks/__init__.py b/src/nwbinspector/checks/__init__.py index b289aa6f..a2bbb1a8 100644 --- a/src/nwbinspector/checks/__init__.py +++ b/src/nwbinspector/checks/__init__.py @@ -49,6 +49,7 @@ check_subject_sex, check_subject_species_exists, check_subject_species_form, + check_subject_weight_format, ) from ._ogen import ( check_optogenetic_stimulus_site_has_optogenetic_series, @@ -148,4 +149,5 @@ "check_spatial_series_dims", "check_spatial_series_degrees_magnitude", "check_rate_is_not_negative", + "check_subject_weight_format", ] diff --git a/src/nwbinspector/checks/_nwbfile_metadata.py b/src/nwbinspector/checks/_nwbfile_metadata.py index 04839503..514d957d 100644 --- a/src/nwbinspector/checks/_nwbfile_metadata.py +++ b/src/nwbinspector/checks/_nwbfile_metadata.py @@ -299,6 +299,24 @@ def check_subject_species_form(subject: Subject) -> Optional[InspectorMessage]: return None +@register_check(importance=Importance.CRITICAL, neurodata_type=Subject) +def check_subject_weight_format(subject: Subject) -> Optional[InspectorMessage]: + """Check if the subject weight has the form '[numeric] [string]'.""" + if subject.weight is None: + return None + + weight_str = str(subject.weight) + if not re.match(r"^\d+(\.\d+)?\s+\w+$", weight_str): + return InspectorMessage( + message="Subject weight should have the form '[numeric] [string]'.", + importance=Importance.CRITICAL, + check_function_name="check_subject_weight_format", + object_type="Subject", + object_name=subject.name, + location="/", + ) + return None + @register_check(importance=Importance.BEST_PRACTICE_SUGGESTION, neurodata_type=ProcessingModule) def check_processing_module_name(processing_module: ProcessingModule) -> Optional[InspectorMessage]: diff --git a/tests/unit_tests/test_nwbfile_metadata.py b/tests/unit_tests/test_nwbfile_metadata.py index 4a9ef027..d5fb295e 100644 --- a/tests/unit_tests/test_nwbfile_metadata.py +++ b/tests/unit_tests/test_nwbfile_metadata.py @@ -22,6 +22,7 @@ check_subject_sex, check_subject_species_exists, check_subject_species_form, + check_subject_weight_format, ) from nwbinspector.checks._nwbfile_metadata import PROCESSING_MODULE_CONFIG from nwbinspector.testing import make_minimal_nwbfile @@ -408,6 +409,46 @@ def test_check_subject_age_iso8601_range_fail_2(): location="/general/subject", ) +def test_check_subject_weight_format_pass(): + subject = Subject(weight="70 kg") + assert check_subject_weight_format(subject) is None + +def test_check_subject_weight_format_invalid_no_unit(): + subject = Subject(weight="70") + assert check_subject_weight_format(subject) == InspectorMessage( + message="Subject weight should have the form '[numeric] [string]'.", + importance=Importance.CRITICAL, + check_function_name="check_subject_weight_format", + object_type="Subject", + object_name=subject.name, + location="/", + ) + +def test_check_subject_weight_format_invalid_no_space(): + subject = Subject(weight="70kg") + assert check_subject_weight_format(subject) == InspectorMessage( + message="Subject weight should have the form '[numeric] [string]'.", + importance=Importance.CRITICAL, + check_function_name="check_subject_weight_format", + object_type="Subject", + object_name=subject.name, + location="/", + ) + +def test_check_subject_weight_format_invalid_non_numeric(): + subject = Subject(weight="seventy kg") + assert check_subject_weight_format(subject) == InspectorMessage( + message="Subject weight should have the form '[numeric] [string]'.", + importance=Importance.CRITICAL, + check_function_name="check_subject_weight_format", + object_type="Subject", + object_name=subject.name, + location="/", + ) + +def test_check_subject_weight_format_none(): + subject = Subject(weight=None) + assert check_subject_weight_format(subject) is None def test_check_subject_proper_age_range_pass(): subject = Subject(subject_id="001", age="P1D/P3D") From dfa709e6b655c4a2224afb7ff8c940d021e76031 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 6 Mar 2025 15:13:17 +0000 Subject: [PATCH 7/9] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/nwbinspector/checks/_nwbfile_metadata.py | 1 + tests/unit_tests/test_nwbfile_metadata.py | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/src/nwbinspector/checks/_nwbfile_metadata.py b/src/nwbinspector/checks/_nwbfile_metadata.py index 514d957d..a19f929c 100644 --- a/src/nwbinspector/checks/_nwbfile_metadata.py +++ b/src/nwbinspector/checks/_nwbfile_metadata.py @@ -299,6 +299,7 @@ def check_subject_species_form(subject: Subject) -> Optional[InspectorMessage]: return None + @register_check(importance=Importance.CRITICAL, neurodata_type=Subject) def check_subject_weight_format(subject: Subject) -> Optional[InspectorMessage]: """Check if the subject weight has the form '[numeric] [string]'.""" diff --git a/tests/unit_tests/test_nwbfile_metadata.py b/tests/unit_tests/test_nwbfile_metadata.py index d5fb295e..62e69a84 100644 --- a/tests/unit_tests/test_nwbfile_metadata.py +++ b/tests/unit_tests/test_nwbfile_metadata.py @@ -409,10 +409,12 @@ def test_check_subject_age_iso8601_range_fail_2(): location="/general/subject", ) + def test_check_subject_weight_format_pass(): subject = Subject(weight="70 kg") assert check_subject_weight_format(subject) is None + def test_check_subject_weight_format_invalid_no_unit(): subject = Subject(weight="70") assert check_subject_weight_format(subject) == InspectorMessage( @@ -424,6 +426,7 @@ def test_check_subject_weight_format_invalid_no_unit(): location="/", ) + def test_check_subject_weight_format_invalid_no_space(): subject = Subject(weight="70kg") assert check_subject_weight_format(subject) == InspectorMessage( @@ -435,6 +438,7 @@ def test_check_subject_weight_format_invalid_no_space(): location="/", ) + def test_check_subject_weight_format_invalid_non_numeric(): subject = Subject(weight="seventy kg") assert check_subject_weight_format(subject) == InspectorMessage( @@ -446,10 +450,12 @@ def test_check_subject_weight_format_invalid_non_numeric(): location="/", ) + def test_check_subject_weight_format_none(): subject = Subject(weight=None) assert check_subject_weight_format(subject) is None + def test_check_subject_proper_age_range_pass(): subject = Subject(subject_id="001", age="P1D/P3D") assert check_subject_proper_age_range(subject) is None From 6608d631d6bb26b46964003f0973c39ef5509cbc Mon Sep 17 00:00:00 2001 From: "jain.anoushka24" Date: Tue, 15 Apr 2025 11:05:16 +0200 Subject: [PATCH 8/9] Fix regex in check_subject_weight_format to allow optional space before unit --- src/nwbinspector/checks/_nwbfile_metadata.py | 2 +- tests/unit_tests/test_nwbfile_metadata.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nwbinspector/checks/_nwbfile_metadata.py b/src/nwbinspector/checks/_nwbfile_metadata.py index 514d957d..45918398 100644 --- a/src/nwbinspector/checks/_nwbfile_metadata.py +++ b/src/nwbinspector/checks/_nwbfile_metadata.py @@ -306,7 +306,7 @@ def check_subject_weight_format(subject: Subject) -> Optional[InspectorMessage]: return None weight_str = str(subject.weight) - if not re.match(r"^\d+(\.\d+)?\s+\w+$", weight_str): + if not re.match(r"^\d+(\.\d+)?\s?\w+$", weight_str): return InspectorMessage( message="Subject weight should have the form '[numeric] [string]'.", importance=Importance.CRITICAL, diff --git a/tests/unit_tests/test_nwbfile_metadata.py b/tests/unit_tests/test_nwbfile_metadata.py index d5fb295e..28c90520 100644 --- a/tests/unit_tests/test_nwbfile_metadata.py +++ b/tests/unit_tests/test_nwbfile_metadata.py @@ -424,7 +424,7 @@ def test_check_subject_weight_format_invalid_no_unit(): location="/", ) -def test_check_subject_weight_format_invalid_no_space(): +def test_check_subject_weight_format_no_space(): subject = Subject(weight="70kg") assert check_subject_weight_format(subject) == InspectorMessage( message="Subject weight should have the form '[numeric] [string]'.", From e1c8b798f8ec1d6042faac7615898a4e820be4f6 Mon Sep 17 00:00:00 2001 From: "jain.anoushka24" Date: Tue, 15 Apr 2025 13:27:00 +0200 Subject: [PATCH 9/9] Add subject weight format documentation to best practices --- docs/best_practices/nwbfile_metadata.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/best_practices/nwbfile_metadata.rst b/docs/best_practices/nwbfile_metadata.rst index cc8319b7..fe3266f5 100644 --- a/docs/best_practices/nwbfile_metadata.rst +++ b/docs/best_practices/nwbfile_metadata.rst @@ -238,6 +238,16 @@ Check function: :py:meth:`~nwbinspector.checks._nwbfile_metadata.check_subject_a +.. _best_practice_subject_weight: + +Subject Weight +~~~~~~~~~~~~~ + +The ``weight`` of a :ref:`nwb-schema:sec-Subject` should have the form '[numeric] [string]', where the numeric part is the weight value and the string part is the unit. For example, "70 kg" or "250 g". This format ensures that both the weight value and its unit are clearly specified and can be easily parsed. + +Check function: :py:meth:`~nwbinspector.checks._nwbfile_metadata.check_subject_weight_format` + + .. _best_practice_subject_dob: Date of Birth