Merge pull request #400 from NeurodataWithoutBorders/fix_empty_string…

…_container

Fix empty string for optional attributes that aren't strings

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Upcoming
 
+### Fixes
+
+* Fixed issue in `check_empty_string_for_optional_attribute` where it would not skip optional non-`str` fields. [#400](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/400)
+
+
+
 # v0.4.29
 
 * Support for Python 3.7 has officially been dropped by the NWB Inspector. Please use Python 3.8 and above. [#380](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/380)

src/nwbinspector/checks/nwb_containers.py

@@ -66,8 +66,9 @@ def check_small_dataset_compression(
 @register_check(importance=Importance.BEST_PRACTICE_SUGGESTION, neurodata_type=NWBContainer)
 def check_empty_string_for_optional_attribute(nwb_container: NWBContainer):
     """
-    Check if any NWBContainer has optional fields that are written as an empty string. These values should just be
-    omitted instead
+    Check if any NWBContainer has optional fields that are written as an empty string.
+
+    These values should just be omitted instead.
 
     Parameters
     ----------
@@ -76,7 +77,9 @@ def check_empty_string_for_optional_attribute(nwb_container: NWBContainer):
     Best Practice: :ref:`best_practice_placeholders`
     """
     docval_args = type(nwb_container).__init__.__docval__["args"]
-    optional_attrs = [arg["name"] for arg in docval_args if "default" in arg and arg["default"] is None]
+    optional_attrs = [
+        arg["name"] for arg in docval_args if arg["type"] is str and "default" in arg and arg["default"] is None
+    ]
     fields = [attr for attr in optional_attrs if getattr(nwb_container, attr) == ""]
     for field in fields:
         yield InspectorMessage(

tests/unit_tests/test_containers.py → tests/unit_tests/test_nwb_containers.py

@@ -7,6 +7,7 @@
 import h5py
 import numpy as np
 from pynwb import NWBContainer, NWBFile
+from pynwb.image import ImageSeries
 
 from nwbinspector import (
     InspectorMessage,
@@ -94,9 +95,9 @@ def test_check_large_dataset_compression_below_20GB(self):
 
 
 def test_hit_check_empty_string_for_optional_attribute():
-    nwb = NWBFile("aa", "aa", datetime.now(), pharmacology="")
+    nwbfile = NWBFile(session_description="aa", identifier="aa", session_start_time=datetime.now(), pharmacology="")
 
-    assert check_empty_string_for_optional_attribute(nwb)[0] == InspectorMessage(
+    assert check_empty_string_for_optional_attribute(nwb_container=nwbfile)[0] == InspectorMessage(
         message='The attribute "pharmacology" is optional and you have supplied an empty string. Improve my omitting '
         "this attribute (in MatNWB or PyNWB) or entering as None (in PyNWB)",
         importance=Importance.BEST_PRACTICE_SUGGESTION,
@@ -108,5 +109,18 @@ def test_hit_check_empty_string_for_optional_attribute():
 
 
 def test_miss_check_empty_string_for_optional_attribute():
-    nwb = NWBFile("aa", "aa", datetime.now())
-    assert check_empty_string_for_optional_attribute(nwb) is None
+    nwbfile = NWBFile(session_description="aa", identifier="aa", session_start_time=datetime.now())
+    assert check_empty_string_for_optional_attribute(nwb_container=nwbfile) is None
+
+
+def test_check_empty_string_for_optional_attribute_skip_non_string():
+    image_series = ImageSeries(
+        name="TestImageSeries",
+        description="Behavior video of animal moving in environment",
+        unit="n.a.",
+        external_file=["test1.mp4", "test2.avi"],
+        format="external",
+        starting_frame=[0, 2],
+        timestamps=[0.0, 0.04, 0.07, 0.1, 0.14, 0.16, 0.21],
+    )  # The `data` field will be created by PyNWB but it will be empty and will otherwise raise warning/error via numpy
+    assert check_empty_string_for_optional_attribute(nwb_container=image_series) is None