Subject or session contains slashes (#570)

* Add s3fs installation to the package setup in CI workflow

* Add checks for session_id and subject_id to prevent slashes in DANDI paths

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update importance level for session_id and subject_id checks to best practice violation

* Add checks for subject and session IDs to prevent slashes

* Enhance documentation for subject ID check with best practice reference

* Update CHANGELOG for upcoming v0.7.0 with new checks for subject_id and session_id slashes

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: bendichter

CHANGELOG.md

@@ -1,5 +1,8 @@
 # v0.6.4 (Upcoming)
 
+### New Checks
+* Added checks to make sure subject_id and session_id do not contain slashes [#570](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/570)
+
 ### Fixes
 * Fixed incorrect data orientation check for SpikeEventSeries [#592](https://github.com/NeurodataWithoutBorders/nwbinspector/issues/592)
 * Fix dimensionality check for SpikeEventSeries validation [#581](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/581)

docs/best_practices/nwbfile_metadata.rst

@@ -71,6 +71,8 @@ Check function: :py:meth:`~nwbinspector.checks._nwbfile_metadata.check_processin
 File Metadata
 -------------
 
+.. _best_practice_session_id:
+
 Session ID
 ~~~~~~~~~~
 
@@ -82,6 +84,12 @@ different processing outputs. In this case, the ``session_id`` should be the sam
 a standard structure for their own naming schemes so that sessions are unique within the lab and the IDs are easily
 human-readable.
 
+The ``session_id`` should not contain slash characters (``/``) as these can cause problems when constructing paths in
+the DANDI archive. If your session IDs normally include slash characters, consider replacing them with hyphens (``-``)
+or underscores (``_``).
+
+Check function: :py:meth:`~nwbinspector.checks._nwbfile_metadata.check_session_id_no_slashes`
+
 .. _best_practice_file_id:
 
 Identifier
@@ -174,7 +182,7 @@ Check function: :py:meth:`~nwbinspector.checks._nwbfile_metadata.check_subject_e
 
 
 
-.. _best_practice_subject_id_exists:
+.. _best_practice_subject_id:
 
 Subject ID
 ~~~~~~~~~~
@@ -185,7 +193,12 @@ not intended for DANDI upload, if the :ref:`nwb-schema:sec-Subject` is specified
 
 In the special case of *in vitro* studies where the 'subject' of scientific interest was not a tissue sample obtained from a living subject but was instead a purified protein, this will be annotated by prepending the keyphrase "protein" to the subject ID; *e.g*, "proteinCaMPARI3". In the case where the *in vitro* experiment is performed on an extracted or cultured biological sample, the other subject attributes (such as age and sex) should be specified as their values at the time the sample was collected.
 
-Check function: :py:meth:`~nwbinspector.checks._nwbfile_metadata.check_subject_id_exists`
+Similar to session IDs, the ``subject_id`` should not contain slash characters (``/``) as these can cause problems when
+constructing paths in the DANDI archive. If your subject IDs normally include slash characters, consider replacing them
+with hyphens (``-``) or underscores (``_``).
+
+Check functions: :py:meth:`~nwbinspector.checks._nwbfile_metadata.check_subject_id_exists` and
+:py:meth:`~nwbinspector.checks._nwbfile_metadata.check_subject_id_no_slashes`
 
 
 

src/nwbinspector/_internal_configs/dandi.inspector_config.yaml

@@ -1,10 +1,12 @@
 CRITICAL:  # All the fields under CRITICAL will be required for dandi validate to pass
   - check_subject_exists
   - check_subject_id_exists
+  - check_subject_id_no_slashes
   - check_subject_sex
   - check_subject_species_exists
   - check_subject_species_form
   - check_subject_age
   - check_subject_proper_age_range
+  - check_session_id_no_slashes
 BEST_PRACTICE_VIOLATION:
   - check_data_orientation  # not 100% accurate, so need to deelevate from CRITICAL to skip it in dandi validate

src/nwbinspector/checks/__init__.py

@@ -42,11 +42,13 @@
     check_institution,
     check_keywords,
     check_processing_module_name,
+    check_session_id_no_slashes,
     check_session_start_time_future_date,
     check_session_start_time_old_date,
     check_subject_age,
     check_subject_exists,
     check_subject_id_exists,
+    check_subject_id_no_slashes,
     check_subject_proper_age_range,
     check_subject_sex,
     check_subject_species_exists,
@@ -114,9 +116,11 @@
     "check_experimenter_exists",
     "check_experiment_description",
     "check_subject_id_exists",
+    "check_subject_id_no_slashes",
     "check_subject_species_exists",
     "check_subject_species_form",
     "check_subject_proper_age_range",
+    "check_session_id_no_slashes",
     "check_session_start_time_future_date",
     "check_processing_module_name",
     "check_session_start_time_old_date",

src/nwbinspector/checks/_nwbfile_metadata.py

@@ -225,7 +225,11 @@ def check_subject_proper_age_range(subject: Subject) -> Optional[InspectorMessag
 
 @register_check(importance=Importance.BEST_PRACTICE_SUGGESTION, neurodata_type=Subject)
 def check_subject_id_exists(subject: Subject) -> Optional[InspectorMessage]:
-    """Check if subject_id is defined."""
+    """
+    Check if subject_id is defined.
+
+    Best Practice: :ref:`best_practice_subject_id`
+    """
     if subject.subject_id is None:
         return InspectorMessage(message="subject_id is missing.")
 
@@ -312,3 +316,39 @@ def check_processing_module_name(processing_module: ProcessingModule) -> Optiona
         )
 
     return None
+
+
+@register_check(importance=Importance.BEST_PRACTICE_VIOLATION, neurodata_type=NWBFile)
+def check_session_id_no_slashes(nwbfile: NWBFile) -> Optional[InspectorMessage]:
+    """
+    Check if session_id contains slash characters, which can cause problems when constructing paths in DANDI.
+
+    Best Practice: :ref:`best_practice_session_id`
+    """
+    if nwbfile.session_id and "/" in nwbfile.session_id:
+        return InspectorMessage(
+            message=(
+                f"The session_id '{nwbfile.session_id}' contains slash character(s) '/', which can cause problems "
+                f"when constructing paths in DANDI. Please replace slashes with another character (e.g., '-' or '_')."
+            )
+        )
+
+    return None
+
+
+@register_check(importance=Importance.BEST_PRACTICE_VIOLATION, neurodata_type=Subject)
+def check_subject_id_no_slashes(subject: Subject) -> Optional[InspectorMessage]:
+    """
+    Check if subject_id contains slash characters, which can cause problems when constructing paths in DANDI.
+
+    Best Practice: :ref:`best_practice_subject_id`
+    """
+    if subject.subject_id and "/" in subject.subject_id:
+        return InspectorMessage(
+            message=(
+                f"The subject_id '{subject.subject_id}' contains slash character(s) '/', which can cause problems "
+                f"when constructing paths in DANDI. Please replace slashes with another character (e.g., '-' or '_')."
+            )
+        )
+
+    return None

tests/test_check_configuration.py

@@ -99,11 +99,13 @@ def test_load_config(self):
                 CRITICAL=[
                     "check_subject_exists",
                     "check_subject_id_exists",
+                    "check_subject_id_no_slashes",
                     "check_subject_sex",
                     "check_subject_species_exists",
                     "check_subject_species_form",
                     "check_subject_age",
                     "check_subject_proper_age_range",
+                    "check_session_id_no_slashes",
                 ],
                 BEST_PRACTICE_VIOLATION=[
                     "check_data_orientation",

tests/unit_tests/test_nwbfile_metadata.py

@@ -13,11 +13,13 @@
     check_institution,
     check_keywords,
     check_processing_module_name,
+    check_session_id_no_slashes,
     check_session_start_time_future_date,
     check_session_start_time_old_date,
     check_subject_age,
     check_subject_exists,
     check_subject_id_exists,
+    check_subject_id_no_slashes,
     check_subject_proper_age_range,
     check_subject_sex,
     check_subject_species_exists,
@@ -565,3 +567,53 @@ def test_check_processing_module_name():
 def test_pass_check_processing_module_name():
     processing_module = ProcessingModule(name="ecephys", description="desc")
     assert check_processing_module_name(processing_module) is None
+
+
+def test_pass_check_session_id_no_slashes():
+    nwbfile = NWBFile(
+        session_description="",
+        identifier=str(uuid4()),
+        session_start_time=datetime.now().astimezone(),
+        session_id="session001",
+    )
+    assert check_session_id_no_slashes(nwbfile) is None
+
+
+def test_check_session_id_with_slashes():
+    nwbfile = NWBFile(
+        session_description="",
+        identifier=str(uuid4()),
+        session_start_time=datetime.now().astimezone(),
+        session_id="session/001",
+    )
+    assert check_session_id_no_slashes(nwbfile) == InspectorMessage(
+        message=(
+            "The session_id 'session/001' contains slash character(s) '/', which can cause problems "
+            "when constructing paths in DANDI. Please replace slashes with another character (e.g., '-' or '_')."
+        ),
+        importance=Importance.BEST_PRACTICE_VIOLATION,
+        check_function_name="check_session_id_no_slashes",
+        object_type="NWBFile",
+        object_name="root",
+        location="/",
+    )
+
+
+def test_pass_check_subject_id_no_slashes():
+    subject = Subject(subject_id="subject001")
+    assert check_subject_id_no_slashes(subject) is None
+
+
+def test_check_subject_id_with_slashes():
+    subject = Subject(subject_id="subject/001")
+    assert check_subject_id_no_slashes(subject) == InspectorMessage(
+        message=(
+            "The subject_id 'subject/001' contains slash character(s) '/', which can cause problems "
+            "when constructing paths in DANDI. Please replace slashes with another character (e.g., '-' or '_')."
+        ),
+        importance=Importance.BEST_PRACTICE_VIOLATION,
+        check_function_name="check_subject_id_no_slashes",
+        object_type="Subject",
+        object_name="subject",
+        location="/general/subject",
+    )