diff --git a/CHANGELOG.md b/CHANGELOG.md index 96e03b2a..8cccbc13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ### New Checks * Added `check_spike_times_without_nans` to detect NaN values in Units spike times, which indicate conversion bugs or unclean array padding. [#689](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/689) +* Added `check_subject_age_reference` to validate that `Subject.age__reference`, when present, is one of the supported values (`"birth"` or `"gestational"`). This catches invalid references in files written by tools that do not enforce the schema constraint. [#250](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/250) ### Improvements * Raised the minimum required PyNWB to `>=4.0` to track the latest PyNWB release. [#708](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/708) diff --git a/docs/best_practices/nwbfile_metadata.rst b/docs/best_practices/nwbfile_metadata.rst index 6cbbeedb..2678fc2e 100644 --- a/docs/best_practices/nwbfile_metadata.rst +++ b/docs/best_practices/nwbfile_metadata.rst @@ -277,7 +277,13 @@ If the precise age is unknown, an age range can be given by "[lower bound]/[uppe that the age is in between 10 and 20 days. If only the lower bound is known, then including only the slash after that lower bound can be used to indicate a missing bound. For instance, "P90Y/" would indicate that the age is 90 years or older. -Check function: :py:meth:`~nwbinspector.checks._nwbfile_metadata.check_subject_age` +The ``age`` is measured relative to a reference point, controlled by the ``age__reference`` field of the +:ref:`nwb-schema:sec-Subject`. The only supported values are ``"birth"`` (the default) and ``"gestational"``. Use +``"gestational"`` when the age is measured from conception rather than from birth, as is common for embryonic or +prenatal preparations. + +Check functions: :py:meth:`~nwbinspector.checks._nwbfile_metadata.check_subject_age` and +:py:meth:`~nwbinspector.checks._nwbfile_metadata.check_subject_age_reference` diff --git a/src/nwbinspector/checks/__init__.py b/src/nwbinspector/checks/__init__.py index 2bf3777a..d7a386a6 100644 --- a/src/nwbinspector/checks/__init__.py +++ b/src/nwbinspector/checks/__init__.py @@ -59,6 +59,7 @@ check_session_start_time_future_date, check_session_start_time_old_date, check_subject_age, + check_subject_age_reference, check_subject_exists, check_subject_id_exists, check_subject_id_no_slashes, @@ -132,6 +133,7 @@ "check_keywords", "check_institution", "check_subject_age", + "check_subject_age_reference", "check_subject_sex", "check_subject_exists", "check_doi_publications", diff --git a/src/nwbinspector/checks/_nwbfile_metadata.py b/src/nwbinspector/checks/_nwbfile_metadata.py index 341294ac..5333714c 100644 --- a/src/nwbinspector/checks/_nwbfile_metadata.py +++ b/src/nwbinspector/checks/_nwbfile_metadata.py @@ -234,6 +234,25 @@ def check_subject_proper_age_range(subject: Subject) -> Optional[InspectorMessag return None +@register_check(importance=Importance.BEST_PRACTICE_SUGGESTION, neurodata_type=Subject) +def check_subject_age_reference(subject: Subject) -> Optional[InspectorMessage]: + """ + Check if the Subject age reference, when specified, is one of the supported options. + + Best Practice: :ref:`best_practice_subject_age` + """ + valid_options = ["birth", "gestational"] + if subject.age__reference is not None and subject.age__reference not in valid_options: + return InspectorMessage( + message=( + f"Subject age reference, '{subject.age__reference}', is not one of the valid options " + f"({valid_options})." + ) + ) + + return None + + @register_check(importance=Importance.CRITICAL, neurodata_type=Subject) def check_subject_id_exists(subject: Subject) -> Optional[InspectorMessage]: """ diff --git a/tests/unit_tests/test_nwbfile_metadata.py b/tests/unit_tests/test_nwbfile_metadata.py index 75c3dbe7..af988d87 100644 --- a/tests/unit_tests/test_nwbfile_metadata.py +++ b/tests/unit_tests/test_nwbfile_metadata.py @@ -27,6 +27,7 @@ check_session_start_time_future_date, check_session_start_time_old_date, check_subject_age, + check_subject_age_reference, check_subject_exists, check_subject_id_exists, check_subject_id_no_slashes, @@ -474,6 +475,43 @@ def test_check_subject_age_with_years_fail(): ) +def test_check_subject_age_reference_default_pass(): + subject = Subject(subject_id="001", age="P1D") + assert check_subject_age_reference(subject) is None + + +def test_check_subject_age_reference_birth_pass(): + subject = Subject(subject_id="001", age="P1D", age__reference="birth") + assert check_subject_age_reference(subject) is None + + +def test_check_subject_age_reference_gestational_pass(): + subject = Subject(subject_id="001", age="P1D", age__reference="gestational") + assert check_subject_age_reference(subject) is None + + +def test_check_subject_age_reference_none_pass(): + # Files written before age__reference existed (or by other tools) may have no reference set. + subject = Subject(subject_id="001", age="P1D") + subject.fields["age__reference"] = None + assert check_subject_age_reference(subject) is None + + +def test_check_subject_age_reference_fail(): + # PyNWB rejects invalid references at construction time, so emulate a file written by another + # tool with an unsupported value by overriding the field after construction. + subject = Subject(subject_id="001", age="P1D") + subject.fields["age__reference"] = "conception" + assert check_subject_age_reference(subject) == InspectorMessage( + message=("Subject age reference, 'conception', is not one of the valid options (['birth', 'gestational'])."), + importance=Importance.BEST_PRACTICE_SUGGESTION, + check_function_name="check_subject_age_reference", + object_type="Subject", + object_name="subject", + location="/general/subject", + ) + + def test_pass_check_subject_species_exists(): subject = Subject(subject_id="001", species="Homo sapiens") assert check_subject_species_exists(subject) is None