From 683c82d98aef4d8f4a86ac64e7a603e395df9d9e Mon Sep 17 00:00:00 2001 From: Cody Baker Date: Mon, 11 Sep 2023 13:42:04 -0400 Subject: [PATCH 1/2] fix issue for asserting empty string on numpy arrays --- src/nwbinspector/checks/nwb_containers.py | 9 +++++--- ...t_containers.py => test_nwb_containers.py} | 22 +++++++++++++++---- 2 files changed, 24 insertions(+), 7 deletions(-) rename tests/unit_tests/{test_containers.py => test_nwb_containers.py} (81%) diff --git a/src/nwbinspector/checks/nwb_containers.py b/src/nwbinspector/checks/nwb_containers.py index 1c22c417c..d6e8af64b 100644 --- a/src/nwbinspector/checks/nwb_containers.py +++ b/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( diff --git a/tests/unit_tests/test_containers.py b/tests/unit_tests/test_nwb_containers.py similarity index 81% rename from tests/unit_tests/test_containers.py rename to tests/unit_tests/test_nwb_containers.py index 4a89fa731..7a15b6efc 100644 --- a/tests/unit_tests/test_containers.py +++ b/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 From 180e81bbd25bb28f2a5a8d513fd6411dbd51726c Mon Sep 17 00:00:00 2001 From: Cody Baker <51133164+CodyCBakerPhD@users.noreply.github.com> Date: Mon, 11 Sep 2023 13:46:14 -0400 Subject: [PATCH 2/2] Update CHANGELOG.md --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a8d63cc21..6bec1c668 100644 --- a/CHANGELOG.md +++ b/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)