Merge branch 'dev' into document_formatter

Author: CodyCBakerPhD

.github/ISSUE_TEMPLATE/bug_report.yml

@@ -61,6 +61,7 @@ body:
         - 3.7
         - 3.8
         - 3.9
+        - 3.10
     validations:
       required: true
   - type: dropdown

.github/workflows/testing.yml

@@ -1,5 +1,9 @@
 name: Testing
-on: pull_request
+on:
+  schedule:
+    - cron: "0 0 * * *"  # daily
+  pull_request:
+
 jobs:
   build-and-test:
     name: Testing using ${{ matrix.os }} with ${{ matrix.python-version }}
@@ -22,7 +26,9 @@ jobs:
           pip install pytest
           pip install pytest-cov
       - name: Install package
-        run: pip install -e .
+        run: |
+          pip install -e .
+          pip install dandi
       - name: Uninstall h5py
         run: pip uninstall -y h5py
       - name: Install ROS3

.pre-commit-config.yaml

@@ -1,12 +1,12 @@
 repos:
 -   repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.2.0
+    rev: v4.3.0
     hooks:
     -   id: check-yaml
     -   id: end-of-file-fixer
     -   id: trailing-whitespace
 -   repo: https://github.com/psf/black
-    rev: 22.3.0
+    rev: 22.6.0
     hooks:
     -   id: black
         exclude: ^docs/

docs/index.rst

@@ -7,16 +7,8 @@ Welcome to the documentation for the NWBInspector!
   :scale: 100 %
   :align: right
 
-NWBInspector is a Python-based package designed to asses the quality of Neurodata Without Borders
-files (NWBFiles) and to suggest improvements for any Best Practice violations that are found.
-
-
-.. note::
-
-    This package is in alpha development; as such, we make every effort towards
-    a stable environment but bugs are known to occur. If you use this software
-    for your own quality assurance purposes and discover any issues throughout
-    the process, please let us know by filing a ticket on our :nwbinspector-issues: page.
+NWBInspector is a Python-based package designed to asses the quality of Neurodata Without Borders (NWB)
+files and based on compliance with Best Practice. This tool is meant as a companion to the PyNWB validator, which checks for strict schema compliance. In contrast, this tool attempts to apply some commonsense rules and heuristics to find data components of a file that pass validation, but are probably incorrect, or suboptimal, or deviate from best practices. In other words, while the PyNWB validator focuses on compliance of the structure of a file with the schema, the inspector focuses on compliance of the actual data with best practices. The NWB Inspector is meant simply as a data review aid. It does not catch all best practice violations, and any warnings it does produce should be checked by a knowledgeable reviewer.
 
 .. toctree::
    :maxdepth: 2

nwbinspector/checks/behavior.py

@@ -1,5 +1,5 @@
 """Checks for types belonging to the pynwb.behavior module."""
-from pynwb.behavior import SpatialSeries
+from pynwb.behavior import SpatialSeries, CompassDirection
 
 from ..register_checks import register_check, Importance, InspectorMessage
 
@@ -11,3 +11,13 @@ def check_spatial_series_dims(spatial_series: SpatialSeries):
         return InspectorMessage(
             message="SpatialSeries should have 1 column (x), 2 columns (x, y), or 3 columns (x, y, z)."
         )
+
+
+@register_check(importance=Importance.BEST_PRACTICE_VIOLATION, neurodata_type=CompassDirection)
+def check_compass_direction_unit(compass_direction: CompassDirection):
+    for spatial_series in compass_direction.spatial_series.values():
+        if spatial_series.unit not in ("degrees", "radians"):
+            yield InspectorMessage(
+                message=f"SpatialSeries objects inside a CompassDirection object should be angular and should have a "
+                f"unit of 'degrees' or 'radians', but '{spatial_series.name}' has units '{spatial_series.unit}'."
+            )

nwbinspector/checks/image_series.py

@@ -14,6 +14,7 @@ def check_image_series_external_file_valid(image_series: ImageSeries):
         return
     nwbfile_path = Path(get_nwbfile_path_from_internal_object(obj=image_series))
     for file_path in image_series.external_file:
+        file_path = file_path.decode() if isinstance(file_path, bytes) else file_path
         if not Path(file_path).is_absolute() and not (nwbfile_path.parent / file_path).exists():
             yield InspectorMessage(
                 message=(
@@ -29,6 +30,7 @@ def check_image_series_external_file_relative(image_series: ImageSeries):
     if image_series.external_file is None:
         return
     for file_path in image_series.external_file:
+        file_path = file_path.decode() if isinstance(file_path, bytes) else file_path
         if Path(file_path).is_absolute():
             yield InspectorMessage(
                 message=(

nwbinspector/checks/nwbfile_metadata.py

@@ -80,8 +80,7 @@ def check_doi_publications(nwbfile: NWBFile):
     if not nwbfile.related_publications:
         return
     for publication in nwbfile.related_publications:
-        if isinstance(publication, bytes):
-            publication = publication.decode()
+        publication = publication.decode() if isinstance(publication, bytes) else publication
         if not any((publication.startswith(valid_start) for valid_start in valid_starts)):
             yield InspectorMessage(
                 message=(

nwbinspector/checks/time_series.py

@@ -86,7 +86,9 @@ def check_missing_unit(time_series: TimeSeries):
 @register_check(importance=Importance.BEST_PRACTICE_VIOLATION, neurodata_type=TimeSeries)
 def check_resolution(time_series: TimeSeries):
     """Check the resolution value of a TimeSeries for proper format (-1.0 or NaN for unknown)."""
-    if time_series.resolution != -1.0 and time_series.resolution <= 0:
+    if time_series.resolution is None or time_series.resolution == -1.0:
+        return
+    if time_series.resolution <= 0:
         return InspectorMessage(
             message=f"'resolution' should use -1.0 or NaN for unknown instead of {time_series.resolution}."
         )

nwbinspector/inspector_tools.py

@@ -13,21 +13,16 @@
 import numpy as np
 
 from .register_checks import InspectorMessage, Importance
-from .utils import FilePathType
-
-try:
-    from importlib.metadata import version
-
-    inspector_version = version("nwbinspector")
-except ModuleNotFoundError:  # Remove the except clause when minimal supported version becomes 3.8
-    from pkg_resources import get_distribution
-
-    inspector_version = get_distribution("nwbinspector").version
+from .utils import FilePathType, get_package_version
 
 
 def get_report_header():
     """Grab basic information from system at time of report generation."""
-    return dict(Timestamp=str(datetime.now().astimezone()), Platform=platform(), NWBInspector_version=inspector_version)
+    return dict(
+        Timestamp=str(datetime.now().astimezone()),
+        Platform=platform(),
+        NWBInspector_version=get_package_version("nwbinspector"),
+    )
 
 
 def _sort_unique_values(unique_values: list, reverse: bool = False):
@@ -82,10 +77,7 @@ class FormatterOptions:
     """Class structure for defining all free attributes for the design of a report format."""
 
     def __init__(
-        self,
-        indent_size: int = 2,
-        indent: Optional[str] = None,
-        section_headers: List[str] = ["=", "-", "~"],
+        self, indent_size: int = 2, indent: Optional[str] = None, section_headers: List[str] = ["=", "-", "~"]
     ):
         # TODO
         # Future custom options could include section break sizes, section-specific indents, etc.

nwbinspector/nwbinspector.py

@@ -8,11 +8,12 @@
 from pathlib import Path
 from collections.abc import Iterable
 from enum import Enum
-from typing import Optional, List
+from typing import Union, Optional, List
 from concurrent.futures import ProcessPoolExecutor, as_completed
 from types import FunctionType
 from warnings import filterwarnings, warn
 from distutils.util import strtobool
+from time import sleep
 
 import click
 import pynwb
@@ -28,7 +29,7 @@
 )
 from .register_checks import InspectorMessage, Importance
 from .tools import get_s3_urls_and_dandi_paths
-from .utils import FilePathType, PathType, OptionalListOfStrings
+from .utils import FilePathType, PathType, OptionalListOfStrings, robust_s3_read, calculate_number_of_cpu
 
 INTERNAL_CONFIGS = dict(dandi=Path(__file__).parent / "internal_configs" / "dandi.inspector_config.yaml")
 
@@ -278,7 +279,7 @@ def inspect_all(
     config: Optional[dict] = None,
     ignore: OptionalListOfStrings = None,
     select: OptionalListOfStrings = None,
-    importance_threshold: Importance = Importance.BEST_PRACTICE_SUGGESTION,
+    importance_threshold: Union[str, Importance] = Importance.BEST_PRACTICE_SUGGESTION,
     n_jobs: int = 1,
     skip_validate: bool = False,
     progress_bar: bool = True,
@@ -305,7 +306,7 @@ def inspect_all(
         Names of functions to skip.
     select: list of strings, optional
         Names of functions to pick out of available checks.
-    importance_threshold : string, optional
+    importance_threshold : string or Importance, optional
         Ignores tests with an assigned importance below this threshold.
         Importance has three levels:
             CRITICAL
@@ -317,6 +318,8 @@ def inspect_all(
         The default is the lowest level, BEST_PRACTICE_SUGGESTION.
     n_jobs : int
         Number of jobs to use in parallel. Set to -1 to use all available resources.
+        This may also be a negative integer x from -2 to -(number_of_cpus - 1) which acts like negative slicing by using
+        all available CPUs minus x.
         Set to 1 (also the default) to disable.
     skip_validate : bool, optional
         Skip the PyNWB validation step. This may be desired for older NWBFiles (< schema version v2.10).
@@ -336,7 +339,11 @@ def inspect_all(
         Common options are 'draft' or 'published'.
         Defaults to the most recent published version, or if not published then the most recent draft version.
     """
+    importance_threshold = (
+        Importance[importance_threshold] if isinstance(importance_threshold, str) else importance_threshold
+    )
     modules = modules or []
+    n_jobs = calculate_number_of_cpu(requested_cpu=n_jobs)
     if progress_bar_options is None:
         progress_bar_options = dict(position=0, leave=False)
         if stream:
@@ -410,9 +417,10 @@ def inspect_nwb(
     config: dict = None,
     ignore: OptionalListOfStrings = None,
     select: OptionalListOfStrings = None,
-    importance_threshold: Importance = Importance.BEST_PRACTICE_SUGGESTION,
-    driver: str = None,
+    importance_threshold: Union[str, Importance] = Importance.BEST_PRACTICE_SUGGESTION,
+    driver: Optional[str] = None,
     skip_validate: bool = False,
+    max_retries: int = 10,
 ) -> List[InspectorMessage]:
     """
     Inspect a NWBFile object and return suggestions for improvements according to best practices.
@@ -431,7 +439,7 @@ def inspect_nwb(
         Names of functions to skip.
     select: list, optional
         Names of functions to pick out of available checks.
-    importance_threshold : string, optional
+    importance_threshold : string or Importance, optional
         Ignores tests with an assigned importance below this threshold.
         Importance has three levels:
             CRITICAL
@@ -446,38 +454,47 @@ def inspect_nwb(
     skip_validate : bool
         Skip the PyNWB validation step. This may be desired for older NWBFiles (< schema version v2.10).
         The default is False, which is also recommended.
+    max_retries : int, optional
+        When using the ros3 driver to stream data from an s3 path, occasional curl issues can result.
+        AWS suggests using iterative retry with an exponential backoff of 0.1 * 2^retries.
+        This sets a hard bound on the number of times to attempt to retry the collection of messages.
+        Defaults to 10 (corresponds to 102.4s maximum delay on final attempt).
     """
+    importance_threshold = (
+        Importance[importance_threshold] if isinstance(importance_threshold, str) else importance_threshold
+    )
     if any(x is not None for x in [config, ignore, select, importance_threshold]):
         checks = configure_checks(
             checks=checks, config=config, ignore=ignore, select=select, importance_threshold=importance_threshold
         )
     nwbfile_path = str(nwbfile_path)
     filterwarnings(action="ignore", message="No cached namespaces found in .*")
     filterwarnings(action="ignore", message="Ignoring cached namespace .*")
+
     with pynwb.NWBHDF5IO(path=nwbfile_path, mode="r", load_namespaces=True, driver=driver) as io:
-        if skip_validate:
+        if not skip_validate:
             validation_errors = pynwb.validate(io=io)
-            if any(validation_errors):
-                for validation_error in validation_errors:
-                    yield InspectorMessage(
-                        message=validation_error.reason,
-                        importance=Importance.PYNWB_VALIDATION,
-                        check_function_name=validation_error.name,
-                        location=validation_error.location,
-                        file_path=nwbfile_path,
-                    )
+            for validation_error in validation_errors:
+                yield InspectorMessage(
+                    message=validation_error.reason,
+                    importance=Importance.PYNWB_VALIDATION,
+                    check_function_name=validation_error.name,
+                    location=validation_error.location,
+                    file_path=nwbfile_path,
+                )
+
         try:
-            nwbfile = io.read()
+            nwbfile = robust_s3_read(command=io.read, max_retries=max_retries)
+            for inspector_message in run_checks(nwbfile=nwbfile, checks=checks):
+                inspector_message.file_path = nwbfile_path
+                yield inspector_message
         except Exception as ex:
             yield InspectorMessage(
                 message=traceback.format_exc(),
                 importance=Importance.ERROR,
-                check_function_name=f"{type(ex)}: {str(ex)}",
+                check_function_name=f"During io.read() - {type(ex)}: {str(ex)}",
                 file_path=nwbfile_path,
             )
-        for inspector_message in run_checks(nwbfile, checks=checks):
-            inspector_message.file_path = nwbfile_path
-            yield inspector_message
 
 
 def run_checks(nwbfile: pynwb.NWBFile, checks: list):
@@ -493,7 +510,7 @@ def run_checks(nwbfile: pynwb.NWBFile, checks: list):
         for nwbfile_object in nwbfile.objects.values():
             if check_function.neurodata_type is None or issubclass(type(nwbfile_object), check_function.neurodata_type):
                 try:
-                    output = check_function(nwbfile_object)
+                    output = robust_s3_read(command=check_function, command_args=[nwbfile_object])
                 # if an individual check fails, include it in the report and continue with the inspection
                 except Exception:
                     output = InspectorMessage(