Merge pull request #218 from NeurodataWithoutBorders/debug_none_names

[Bug]: Fix None fields with config

Author: CodyCBakerPhD

nwbinspector/internal_configs/dandi.inspector_config.yaml

@@ -2,7 +2,7 @@ CRITICAL:  # All the fields under CRITICAL will be required for dandi validate t
   - check_subject_exists
   - check_subject_id_exists
 BEST_PRACTICE_VIOLATION:
-  - check_subject_sex # these are planned to be elevated to CRITICAL when requried for DANDI validate
-  - check_subject_species # these are planned to be elevated to CRITICAL when requried for DANDI validate
-  - check_subject_age # these are planned to be elevated to CRITICAL when requried for DANDI validate
+  - check_subject_sex # these are planned to be elevated to CRITICAL when required for DANDI validate
+  - check_subject_species # these are planned to be elevated to CRITICAL when required for DANDI validate
+  - check_subject_age # these are planned to be elevated to CRITICAL when required for DANDI validate
   - check_data_orientation  # not 100% accurate, so need to deelevate from CRITICAL to skip it in dandi validate

nwbinspector/nwbinspector.py

@@ -37,7 +37,7 @@
 class InspectorOutputJSONEncoder(json.JSONEncoder):
     """Custom JSONEncoder for the NWBInspector."""
 
-    def default(self, o):
+    def default(self, o):  # noqa D102
         if isinstance(o, InspectorMessage):
             return o.__dict__
         if isinstance(o, Enum):
@@ -53,25 +53,41 @@ def validate_config(config: dict):
     jsonschema.validate(instance=config, schema=schema)
 
 
-def copy_function(function):
+def _copy_function(function):
     """
-    Return a copy of a function so that internal attributes can be adjusted without changing the original function.
+    Copy the core parts of a given function, excluding wrappers, then return a new function.
 
-    Required to ensure our configuration of functions in the registry does not effect the registry itself.
-
-    Taken from
+    Based off of
     https://stackoverflow.com/questions/6527633/how-can-i-make-a-deepcopy-of-a-function-in-python/30714299#30714299
     """
-    if getattr(function, "__wrapped__", False):
-        function = function.__wrapped__
     copied_function = FunctionType(
         function.__code__, function.__globals__, function.__name__, function.__defaults__, function.__closure__
     )
-    # in case f was given attrs (note this dict is a shallow copy):
+
+    # in case f was given attrs (note this dict is a shallow copy)
     copied_function.__dict__.update(function.__dict__)
     return copied_function
 
 
+def copy_check(function):
+    """
+    Copy a check function so that internal attributes can be adjusted without changing the original function.
+
+    Required to ensure our configuration of functions in the registry does not effect the registry itself.
+
+    Also copies the wrapper for auto-parsing ressults,
+    see https://github.com/NeurodataWithoutBorders/nwbinspector/pull/218 for explanation.
+
+    Taken from
+    https://stackoverflow.com/questions/6527633/how-can-i-make-a-deepcopy-of-a-function-in-python/30714299#30714299
+    """
+    if getattr(function, "__wrapped__", False):
+        check_function = function.__wrapped__
+    copied_function = _copy_function(function)
+    copied_function.__wrapped__ = _copy_function(check_function)
+    return copied_function
+
+
 def load_config(filepath_or_keyword: PathType) -> dict:
     """
     Load a config dictionary either via keyword search of the internal configs, or an explicit filepath.
@@ -131,7 +147,7 @@ def configure_checks(
         checks_out = []
         ignore = ignore or []
         for check in checks:
-            mapped_check = copy_function(check)
+            mapped_check = copy_check(check)
             for importance_name, func_names in config.items():
                 if check.__name__ in func_names:
                     if importance_name == "SKIP":

requirements.txt

@@ -2,3 +2,6 @@ pynwb
 PyYAML
 jsonschema
 packaging
+natsort
+click
+tqdm

setup.py

@@ -1,8 +1,11 @@
 from setuptools import setup, find_packages
+from pathlib import Path
 
-# Get the long description from the README file
-with open("README.md", "r") as f:
+root = Path(__file__).parent
+with open(root / "README.md", "r") as f:
     long_description = f.read()
+with open(root / "requirements.txt") as f:
+    install_requires = f.readlines()
 setup(
     name="nwbinspector",
     version="0.4.9",
@@ -14,7 +17,7 @@
     packages=find_packages(),
     include_package_data=True,
     url="https://github.com/NeurodataWithoutBorders/nwbinspector",
-    install_requires=["pynwb", "natsort", "click", "PyYAML", "jsonschema", "tqdm"],
+    install_requires=install_requires,
     extras_require=dict(dandi=["dandi>=0.39.2"]),
     entry_points={"console_scripts": ["nwbinspector=nwbinspector.nwbinspector:inspect_all_cli"]},
     license="BSD-3-Clause",

tests/test_check_configuration.py

@@ -9,7 +9,7 @@
     check_timestamps_match_first_dimension,
     available_checks,
 )
-from nwbinspector.nwbinspector import validate_config, configure_checks, copy_function, load_config
+from nwbinspector.nwbinspector import validate_config, configure_checks, _copy_function, load_config
 
 
 class TestCheckConfiguration(TestCase):
@@ -24,7 +24,7 @@ def setUpClass(cls):
 
     def test_safe_check_copy(self):
         initial_importance = available_checks[0].importance
-        changed_check = copy_function(function=available_checks[0])
+        changed_check = _copy_function(function=available_checks[0])
         if initial_importance is Importance.CRITICAL:
             changed_importance = Importance.BEST_PRACTICE_SUGGESTION
         else:

tests/test_inspector.py

@@ -17,8 +17,10 @@
     check_regular_timestamps,
     check_data_orientation,
     check_timestamps_match_first_dimension,
+    check_subject_exists,
+    load_config,
 )
-from nwbinspector.nwbinspector import inspect_all, inspect_nwb
+from nwbinspector import inspect_all, inspect_nwb
 from nwbinspector.register_checks import Severity, InspectorMessage, register_check
 from nwbinspector.utils import FilePathType, is_module_installed
 from nwbinspector.tools import make_minimal_nwbfile
@@ -144,7 +146,7 @@ def assertLogFileContentsEqual(
                     if ".nwb" in test_line:
                         # Transform temporary testing path and formatted to hardcoded fake path
                         str_loc = test_line.find(".nwb")
-                        correction_str = test_line.replace(test_line[5 : str_loc - 8], "./")
+                        correction_str = test_line.replace(test_line[5 : str_loc - 8], "./")  # noqa: E203 (black)
                         test_file_lines[line_number] = correction_str
                 self.assertEqual(first=test_file_lines[skip_first_n_lines:-1], second=true_file_lines)
 
@@ -501,6 +503,71 @@ def test_inspect_nwb_manual_iteration_stop(self):
         with self.assertRaises(expected_exception=StopIteration):
             next(generator)
 
+    def test_inspect_nwb_dandi_config(self):
+        config_checks = [check_subject_exists] + self.checks
+        test_results = list(
+            inspect_nwb(
+                nwbfile_path=self.nwbfile_paths[0],
+                checks=config_checks,
+                config=load_config(filepath_or_keyword="dandi"),
+            )
+        )
+        true_results = [
+            InspectorMessage(
+                message="Subject is missing.",
+                importance=Importance.BEST_PRACTICE_SUGGESTION,
+                check_function_name="check_subject_exists",
+                object_type="NWBFile",
+                object_name="root",
+                location="/",
+                file_path=self.nwbfile_paths[0],
+            ),
+            InspectorMessage(
+                message="data is not compressed. Consider enabling compression when writing a dataset.",
+                importance=Importance.BEST_PRACTICE_SUGGESTION,
+                check_function_name="check_small_dataset_compression",
+                object_type="TimeSeries",
+                object_name="test_time_series_1",
+                location="/acquisition/test_time_series_1",
+                file_path=self.nwbfile_paths[0],
+            ),
+            InspectorMessage(
+                message=(
+                    "TimeSeries appears to have a constant sampling rate. "
+                    "Consider specifying starting_time=1.2 and rate=2.0 instead of timestamps."
+                ),
+                importance=Importance.BEST_PRACTICE_VIOLATION,
+                check_function_name="check_regular_timestamps",
+                object_type="TimeSeries",
+                object_name="test_time_series_2",
+                location="/acquisition/test_time_series_2",
+                file_path=self.nwbfile_paths[0],
+            ),
+            InspectorMessage(
+                message=(
+                    "Data may be in the wrong orientation. "
+                    "Time should be in the first dimension, and is usually the longest dimension. "
+                    "Here, another dimension is longer."
+                ),
+                importance=Importance.CRITICAL,
+                check_function_name="check_data_orientation",
+                object_type="SpatialSeries",
+                object_name="my_spatial_series",
+                location="/processing/behavior/Position/my_spatial_series",
+                file_path=self.nwbfile_paths[0],
+            ),
+            InspectorMessage(
+                message="The length of the first dimension of data does not match the length of timestamps.",
+                importance=Importance.CRITICAL,
+                check_function_name="check_timestamps_match_first_dimension",
+                object_type="TimeSeries",
+                object_name="test_time_series_3",
+                location="/acquisition/test_time_series_3",
+                file_path=self.nwbfile_paths[0],
+            ),
+        ]
+        self.assertCountEqual(first=test_results, second=true_results)
+
 
 @pytest.mark.skipif(not HAVE_ROS3 or not HAVE_DANDI, reason="Needs h5py setup with ROS3.")
 def test_dandiset_streaming():

tests/unit_tests/test_tables.py

@@ -229,8 +229,8 @@ def test_check_single_row_pass():
 
 def test_check_single_row_ignore_units():
     table = Units(
-        name="Units",  # default name when building through nwbfile
-    )
+        name="Units",
+    )  # default name when building through nwbfile
     table.add_unit(spike_times=[1, 2, 3])
     assert check_single_row(table=table) is None