From ffe2624471a4bc12e0e49dcbcf1fa81888910b4d Mon Sep 17 00:00:00 2001 From: Cody Baker Date: Tue, 6 Sep 2022 16:08:11 -0400 Subject: [PATCH 1/4] fix 260 --- docs/developer_guide.rst | 14 ++++++++++ tests/test_inspector.py | 34 ++++++++++++----------- tests/unit_tests/test_time_series.py | 40 ++++++++++++++++------------ 3 files changed, 56 insertions(+), 32 deletions(-) diff --git a/docs/developer_guide.rst b/docs/developer_guide.rst index 653307977..7a37b0ab6 100644 --- a/docs/developer_guide.rst +++ b/docs/developer_guide.rst @@ -35,3 +35,17 @@ Then, all that is needed for this to be automatically included when you run the the modules flag ``-m`` or ``--modules`` along with the name of your module that contains the custom check. If using the library instead, you need only import the ``available_checks`` global variable from your own submodules, or otherwise import your check functions after importing the ``nwbinspector`` in your ``__init__.py``. + + +Disable Tests That Require Network Connection +--------------------------------------------- + +Some of the tests in the suite require internet connectivity both to and from the DANDI archive S3 bucket. +If this is failing for some reason, you can explicitly control all related tests by setting the environment variable +``DANDI_TESTS_NONETWORK`` to some value able to be parsed by ``distutils.util.str2tool``. For example, to disable them on +a linux system, run + +.. code-block:: + export DANDI_TESTS_NONETWORK=1 + +in your environment before running ``pytest``. \ No newline at end of file diff --git a/tests/test_inspector.py b/tests/test_inspector.py index 45112e026..a28d773ea 100644 --- a/tests/test_inspector.py +++ b/tests/test_inspector.py @@ -5,6 +5,7 @@ from pathlib import Path from unittest import TestCase from datetime import datetime +from distutils.util import strtobool import numpy as np from pynwb import NWBFile, NWBHDF5IO, TimeSeries @@ -27,19 +28,22 @@ from nwbinspector.utils import FilePathType, is_module_installed from nwbinspector.tools import make_minimal_nwbfile - -try: - with NWBHDF5IO( - path="https://dandiarchive.s3.amazonaws.com/blobs/11e/c89/11ec8933-1456-4942-922b-94e5878bb991", - mode="r", - load_namespaces=True, - driver="ros3", - ) as io: - nwbfile = io.read() - HAVE_ROS3 = True -except ValueError: # ValueError: h5py was built without ROS3 support, can't use ros3 driver - HAVE_ROS3 = False +DANDI_TESTS_NONETWORK = os.environ.get("DANDI_TESTS_NONETWORK", "") +NO_NETWORK = strtobool(DANDI_TESTS_NONETWORK) if DANDI_TESTS_NONETWORK != "" else False +if not NO_NETWORK: + try: + with NWBHDF5IO( + path="https://dandiarchive.s3.amazonaws.com/blobs/11e/c89/11ec8933-1456-4942-922b-94e5878bb991", + mode="r", + load_namespaces=True, + driver="ros3", + ) as io: + nwbfile = io.read() + HAVE_ROS3 = True + except ValueError: # ValueError: h5py was built without ROS3 support, can't use ros3 driver + HAVE_ROS3 = False HAVE_DANDI = is_module_installed("dandi") +DISABLE_STREAMING_TESTS = NO_NETWORK or not HAVE_ROS3 or not HAVE_DANDI def add_big_dataset_no_compression(nwbfile: NWBFile): @@ -571,7 +575,7 @@ def test_inspect_nwb_dandi_config(self): self.assertCountEqual(first=test_results, second=true_results) -@pytest.mark.skipif(not HAVE_ROS3 or not HAVE_DANDI, reason="Needs h5py setup with ROS3.") +@pytest.mark.skipif(DISABLE_STREAMING_TESTS, reason="Needs h5py setup with ROS3.") def test_dandiset_streaming(): messages = list(inspect_all(path="000126", select=["check_subject_species_exists"], stream=True)) assert messages[0] == InspectorMessage( @@ -585,7 +589,7 @@ def test_dandiset_streaming(): ) -@pytest.mark.skipif(not HAVE_ROS3 or not HAVE_DANDI, reason="Needs h5py setup with ROS3.") +@pytest.mark.skipif(DISABLE_STREAMING_TESTS, reason="Needs h5py setup with ROS3.") def test_dandiset_streaming_parallel(): messages = list(inspect_all(path="000126", select=["check_subject_species_exists"], stream=True, n_jobs=2)) assert messages[0] == InspectorMessage( @@ -599,7 +603,7 @@ def test_dandiset_streaming_parallel(): ) -@pytest.mark.skipif(not HAVE_ROS3 or not HAVE_DANDI, reason="Needs h5py setup with ROS3.") +@pytest.mark.skipif(DISABLE_STREAMING_TESTS, reason="Needs h5py setup with ROS3.") class TestStreamingCLI(TestCase): @classmethod def setUpClass(cls): diff --git a/tests/unit_tests/test_time_series.py b/tests/unit_tests/test_time_series.py index 3428d566e..962a47f4c 100644 --- a/tests/unit_tests/test_time_series.py +++ b/tests/unit_tests/test_time_series.py @@ -1,9 +1,10 @@ -from packaging import version -from time import sleep +import os +from distutils.util import strtobool import numpy as np import pynwb import pytest +from packaging import version from nwbinspector import ( InspectorMessage, @@ -15,20 +16,25 @@ check_missing_unit, check_resolution, ) -from nwbinspector.utils import get_package_version, robust_s3_read - -try: - # Test ros3 on sub-YutaMouse54/sub-YutaMouse54_ses-YutaMouse54-160630_behavior+ecephys.nwb from #3 - with pynwb.NWBHDF5IO( - path="https://dandiarchive.s3.amazonaws.com/blobs/f03/18e/f0318e30-4f4f-466d-a8e9-a962863e3081", - mode="r", - load_namespaces=True, - driver="ros3", - ) as io: - nwbfile = io.read() - HAVE_ROS3 = True -except ValueError: # ValueError: h5py was built without ROS3 support, can't use ros3 driver - HAVE_ROS3 = False +from nwbinspector.utils import get_package_version, robust_s3_read, is_module_installed + +DANDI_TESTS_NONETWORK = os.environ.get("DANDI_TESTS_NONETWORK", "") +NO_NETWORK = strtobool(DANDI_TESTS_NONETWORK) if DANDI_TESTS_NONETWORK != "" else False +if not NO_NETWORK: + try: + # Test ros3 on sub-YutaMouse54/sub-YutaMouse54_ses-YutaMouse54-160630_behavior+ecephys.nwb from #3 + with pynwb.NWBHDF5IO( + path="https://dandiarchive.s3.amazonaws.com/blobs/f03/18e/f0318e30-4f4f-466d-a8e9-a962863e3081", + mode="r", + load_namespaces=True, + driver="ros3", + ) as io: + nwbfile = io.read() + HAVE_ROS3 = True + except ValueError: # ValueError: h5py was built without ROS3 support, can't use ros3 driver + HAVE_ROS3 = False +HAVE_DANDI = is_module_installed("dandi") +DISABLE_STREAMING_TESTS = NO_NETWORK or not HAVE_ROS3 or not HAVE_DANDI def test_check_regular_timestamps(): @@ -179,7 +185,7 @@ def test_check_unknown_resolution_pass(): @pytest.mark.skipif( - not HAVE_ROS3 or get_package_version("hdmf") >= version.parse("3.3.1"), + DISABLE_STREAMING_TESTS or get_package_version("hdmf") >= version.parse("3.3.1"), reason="Needs h5py setup with ROS3, as well as 'hdmf<3.3.1'.", ) def test_check_none_matnwb_resolution_pass(): From 9f31ca7225455d768d66f7a1c3309dd402a0a959 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 6 Sep 2022 20:09:35 +0000 Subject: [PATCH 2/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- docs/developer_guide.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/developer_guide.rst b/docs/developer_guide.rst index 7a37b0ab6..aa09daf9e 100644 --- a/docs/developer_guide.rst +++ b/docs/developer_guide.rst @@ -47,5 +47,5 @@ a linux system, run .. code-block:: export DANDI_TESTS_NONETWORK=1 - -in your environment before running ``pytest``. \ No newline at end of file + +in your environment before running ``pytest``. From 23760e62542218d2e32461b190d4eec403d35422 Mon Sep 17 00:00:00 2001 From: CodyCBakerPhD Date: Thu, 15 Sep 2022 19:49:39 +0000 Subject: [PATCH 3/4] cleanup/generalize --- nwbinspector/testing.py | 35 ++++++++++++++++++++++++++++ nwbinspector/tools.py | 21 ++++++++++++++++- tests/test_inspector.py | 23 ++++-------------- tests/unit_tests/test_time_series.py | 28 ++++------------------ 4 files changed, 64 insertions(+), 43 deletions(-) create mode 100644 nwbinspector/testing.py diff --git a/nwbinspector/testing.py b/nwbinspector/testing.py new file mode 100644 index 000000000..35561911e --- /dev/null +++ b/nwbinspector/testing.py @@ -0,0 +1,35 @@ +"""Helper functions for internal use across the testing suite.""" +import os +from distutils.util import strtobool +from typing import Tuple, Optional + +from .tools import check_streaming_enabled +from .utils import is_module_installed + + +def check_streaming_tests_enabled() -> Tuple[bool, Optional[str]]: + """ + General purpose helper for determining if the testing environment can support S3 DANDI streaming. + + Returns the boolean status of the check and, if False, provides a string reason for the failure for the user to + utilize as they please (raise an error or warning with that message, print it, or ignore it). + """ + failure_reason = "" + + environment_skip_flag = os.environ.get("NWBI_SKIP_NETWORK_TESTS", "") + environment_skip_flag_bool = ( + strtobool(os.environ.get("NWBI_SKIP_NETWORK_TESTS", "")) if environment_skip_flag != "" else False + ) + if environment_skip_flag_bool: + failure_reason += "Environmental variable set to skip network tets." + + streaming_enabled, streaming_failure_reason = check_streaming_enabled() + if not streaming_enabled: + failure_reason += streaming_failure_reason + + have_dandi = is_module_installed("dandi") + if not have_dandi: + failure_reason += "The DANDI package is not installed on the system." + + failure_reason = None if failure_reason == "" else failure_reason + return streaming_enabled and not environment_skip_flag_bool and have_dandi, failure_reason diff --git a/nwbinspector/tools.py b/nwbinspector/tools.py index c6fec5cc9..0c15e9760 100644 --- a/nwbinspector/tools.py +++ b/nwbinspector/tools.py @@ -2,9 +2,12 @@ import re from uuid import uuid4 from datetime import datetime -from typing import Optional, Dict +from typing import Optional, Dict, Tuple from concurrent.futures import ProcessPoolExecutor, as_completed +from urllib import request +from warnings import warn +import h5py from pynwb import NWBFile from .utils import is_module_installed, calculate_number_of_cpu @@ -75,3 +78,19 @@ def _get_content_url_and_path(asset, follow_redirects: int = 1, strip_query: boo Must be globally defined (not as a part of get_s3_urls..) in order to be pickled. """ return {asset.get_content_url(follow_redirects=1, strip_query=True): asset.path} + + +def check_streaming_enabled() -> Tuple[bool, Optional[str]]: + """ + General purpose helper for determining if the environment can support S3 DANDI streaming. + + Returns the boolean status of the check and, if False, provides a string reason for the failure for the user to + utilize as they please (raise an error or warning with that message, print it, or ignore it). + """ + try: + request.urlopen("https://dandiarchive.s3.amazonaws.com/ros3test.nwb", timeout=1) + except request.URLError: + return False, "Internet access to DANDI failed." + if "ros3" not in h5py.registered_drivers(): + return False, "ROS3 driver not installed." + return True, None diff --git a/tests/test_inspector.py b/tests/test_inspector.py index a28d773ea..d3f02bb47 100644 --- a/tests/test_inspector.py +++ b/tests/test_inspector.py @@ -5,7 +5,6 @@ from pathlib import Path from unittest import TestCase from datetime import datetime -from distutils.util import strtobool import numpy as np from pynwb import NWBFile, NWBHDF5IO, TimeSeries @@ -25,25 +24,11 @@ ) 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.testing import check_streaming_tests_enabled from nwbinspector.tools import make_minimal_nwbfile +from nwbinspector.utils import FilePathType -DANDI_TESTS_NONETWORK = os.environ.get("DANDI_TESTS_NONETWORK", "") -NO_NETWORK = strtobool(DANDI_TESTS_NONETWORK) if DANDI_TESTS_NONETWORK != "" else False -if not NO_NETWORK: - try: - with NWBHDF5IO( - path="https://dandiarchive.s3.amazonaws.com/blobs/11e/c89/11ec8933-1456-4942-922b-94e5878bb991", - mode="r", - load_namespaces=True, - driver="ros3", - ) as io: - nwbfile = io.read() - HAVE_ROS3 = True - except ValueError: # ValueError: h5py was built without ROS3 support, can't use ros3 driver - HAVE_ROS3 = False -HAVE_DANDI = is_module_installed("dandi") -DISABLE_STREAMING_TESTS = NO_NETWORK or not HAVE_ROS3 or not HAVE_DANDI +DISABLE_STREAMING_TESTS, DISABLE_STREAMING_TESTS_REASON = check_streaming_tests_enabled() def add_big_dataset_no_compression(nwbfile: NWBFile): @@ -603,7 +588,7 @@ def test_dandiset_streaming_parallel(): ) -@pytest.mark.skipif(DISABLE_STREAMING_TESTS, reason="Needs h5py setup with ROS3.") +@pytest.mark.skipif(DISABLE_STREAMING_TESTS, reason=DISABLE_STREAMING_TESTS_REASON or "") class TestStreamingCLI(TestCase): @classmethod def setUpClass(cls): diff --git a/tests/unit_tests/test_time_series.py b/tests/unit_tests/test_time_series.py index 962a47f4c..16299e9f8 100644 --- a/tests/unit_tests/test_time_series.py +++ b/tests/unit_tests/test_time_series.py @@ -1,6 +1,3 @@ -import os -from distutils.util import strtobool - import numpy as np import pynwb import pytest @@ -16,25 +13,10 @@ check_missing_unit, check_resolution, ) -from nwbinspector.utils import get_package_version, robust_s3_read, is_module_installed - -DANDI_TESTS_NONETWORK = os.environ.get("DANDI_TESTS_NONETWORK", "") -NO_NETWORK = strtobool(DANDI_TESTS_NONETWORK) if DANDI_TESTS_NONETWORK != "" else False -if not NO_NETWORK: - try: - # Test ros3 on sub-YutaMouse54/sub-YutaMouse54_ses-YutaMouse54-160630_behavior+ecephys.nwb from #3 - with pynwb.NWBHDF5IO( - path="https://dandiarchive.s3.amazonaws.com/blobs/f03/18e/f0318e30-4f4f-466d-a8e9-a962863e3081", - mode="r", - load_namespaces=True, - driver="ros3", - ) as io: - nwbfile = io.read() - HAVE_ROS3 = True - except ValueError: # ValueError: h5py was built without ROS3 support, can't use ros3 driver - HAVE_ROS3 = False -HAVE_DANDI = is_module_installed("dandi") -DISABLE_STREAMING_TESTS = NO_NETWORK or not HAVE_ROS3 or not HAVE_DANDI +from nwbinspector.testing import check_streaming_tests_enabled +from nwbinspector.utils import get_package_version, robust_s3_read + +DISABLE_STREAMING_TESTS, DISABLE_STREAMING_TESTS_REASON = check_streaming_tests_enabled() def test_check_regular_timestamps(): @@ -186,7 +168,7 @@ def test_check_unknown_resolution_pass(): @pytest.mark.skipif( DISABLE_STREAMING_TESTS or get_package_version("hdmf") >= version.parse("3.3.1"), - reason="Needs h5py setup with ROS3, as well as 'hdmf<3.3.1'.", + reason=f"{DISABLE_STREAMING_TESTS_REASON}. Also needs 'hdmf<3.3.1'.", ) def test_check_none_matnwb_resolution_pass(): """ From b7bb2d74dff2ee25013cd94555a7dcd0036e827f Mon Sep 17 00:00:00 2001 From: CodyCBakerPhD Date: Thu, 15 Sep 2022 20:17:31 +0000 Subject: [PATCH 4/4] fix docs --- docs/developer_guide.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/developer_guide.rst b/docs/developer_guide.rst index aa09daf9e..894b3b2db 100644 --- a/docs/developer_guide.rst +++ b/docs/developer_guide.rst @@ -42,10 +42,10 @@ Disable Tests That Require Network Connection Some of the tests in the suite require internet connectivity both to and from the DANDI archive S3 bucket. If this is failing for some reason, you can explicitly control all related tests by setting the environment variable -``DANDI_TESTS_NONETWORK`` to some value able to be parsed by ``distutils.util.str2tool``. For example, to disable them on +``NWBI_SKIP_NETWORK_TESTS`` to some value able to be parsed by ``distutils.util.str2tool``. For example, to disable them on a linux system, run .. code-block:: - export DANDI_TESTS_NONETWORK=1 + export NWBI_SKIP_NETWORK_TESTS=1 in your environment before running ``pytest``.