diff --git a/.github/workflows/all_os_versions.txt b/.github/workflows/all_os_versions.txt new file mode 100644 index 00000000..2abb0001 --- /dev/null +++ b/.github/workflows/all_os_versions.txt @@ -0,0 +1 @@ +["ubuntu-latest", "macos-latest", "windows-latest"] diff --git a/.github/workflows/all_python_versions.txt b/.github/workflows/all_python_versions.txt new file mode 100644 index 00000000..4daf8ed8 --- /dev/null +++ b/.github/workflows/all_python_versions.txt @@ -0,0 +1 @@ +["3.10", "3.13"] diff --git a/.github/workflows/dailies.yml b/.github/workflows/dailies.yml index 849b165e..9a5fe795 100644 --- a/.github/workflows/dailies.yml +++ b/.github/workflows/dailies.yml @@ -7,10 +7,26 @@ on: jobs: + load_python_and_os_versions: + runs-on: ubuntu-latest + outputs: + ALL_PYTHON_VERSIONS: ${{ steps.load_python_versions.outputs.python_versions }} + ALL_OS_VERSIONS: ${{ steps.load_os_versions.outputs.os_versions }} + steps: + - uses: actions/checkout@v4 + - id: load_python_versions + run: echo "python_versions=$(cat ./.github/workflows/all_python_versions.txt)" >> "$GITHUB_OUTPUT" + - id: load_os_versions + run: echo "os_versions=$(cat ./.github/workflows/all_os_versions.txt)" >> "$GITHUB_OUTPUT" + run-daily-tests: + needs: load_python_and_os_versions uses: neurodatawithoutborders/nwbinspector/.github/workflows/testing.yml@dev secrets: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + with: + python-versions: ${{ needs.load_python_and_os_versions.outputs.ALL_PYTHON_VERSIONS }} + os-versions: ${{ needs.load_python_and_os_versions.outputs.ALL_OS_VERSIONS }} run-daily-doc-link-checks: uses: neurodatawithoutborders/nwbinspector/.github/workflows/doc-link-checks.yml@dev @@ -18,24 +34,50 @@ jobs: test-dandi-latest: uses: neurodatawithoutborders/nwbinspector/.github/workflows/dandi-release.yml@dev - test-dandi-dev: - uses: neurodatawithoutborders/nwbinspector/.github/workflows/dandi-dev.yml@dev + notify-tests-failure: + runs-on: ubuntu-latest + needs: [run-daily-tests] + if: ${{ always() && needs.run-daily-tests.result == 'failure' }} + steps: + - uses: dawidd6/action-send-mail@v3 + with: + server_address: smtp.gmail.com + server_port: 465 + username: ${{ secrets.MAIL_USERNAME }} + password: ${{ secrets.MAIL_PASSWORD }} + subject: NWB Inspector Daily Failure - tests + to: rly@lbl.gov + from: NWB Inspector + body: "The daily test workflow failed: please check status at https://github.com/NeurodataWithoutBorders/nwbinspector/actions/workflows/dailies.yml" - test-dandi-dev-live: - uses: neurodatawithoutborders/nwbinspector/.github/workflows/dandi-dev-live-services.yml@dev + notify-doc-link-checks-failure: + runs-on: ubuntu-latest + needs: [run-daily-doc-link-checks] + if: ${{ always() && needs.run-daily-doc-link-checks.result == 'failure' }} + steps: + - uses: dawidd6/action-send-mail@v3 + with: + server_address: smtp.gmail.com + server_port: 465 + username: ${{ secrets.MAIL_USERNAME }} + password: ${{ secrets.MAIL_PASSWORD }} + subject: NWB Inspector Daily Failure - doc link checks + to: rly@lbl.gov + from: NWB Inspector + body: "The daily documentation link check failed: please check status at https://github.com/NeurodataWithoutBorders/nwbinspector/actions/workflows/dailies.yml" - notify: + notify-dandi-latest-failure: runs-on: ubuntu-latest - needs: [run-daily-tests, run-daily-doc-link-checks, test-dandi-latest, test-dandi-dev, test-dandi-dev-live] - if: failure() + needs: [test-dandi-latest] + if: ${{ always() && needs.test-dandi-latest.result == 'failure' }} steps: - uses: dawidd6/action-send-mail@v3 with: server_address: smtp.gmail.com - server_port: 465 # TSL + server_port: 465 username: ${{ secrets.MAIL_USERNAME }} password: ${{ secrets.MAIL_PASSWORD }} - subject: NWB Inspector Daily Failure - to: smprince@lbl.gov, rly@lbl.gov + subject: NWB Inspector Daily Failure - DANDI latest release + to: rly@lbl.gov from: NWB Inspector - body: "The daily workflow for the NWB Inspector failed: please check status at https://github.com/NeurodataWithoutBorders/nwbinspector/actions/workflows/dailies.yml" + body: "The daily test against the latest released DANDI failed: please check status at https://github.com/NeurodataWithoutBorders/nwbinspector/actions/workflows/dailies.yml" diff --git a/.github/workflows/deploy-tests.yml b/.github/workflows/deploy-tests.yml index 56808ec7..8b3a5f47 100644 --- a/.github/workflows/deploy-tests.yml +++ b/.github/workflows/deploy-tests.yml @@ -9,6 +9,18 @@ concurrency: jobs: + load_python_and_os_versions: + runs-on: ubuntu-latest + outputs: + ALL_PYTHON_VERSIONS: ${{ steps.load_python_versions.outputs.python_versions }} + ALL_OS_VERSIONS: ${{ steps.load_os_versions.outputs.os_versions }} + steps: + - uses: actions/checkout@v4 + - id: load_python_versions + run: echo "python_versions=$(cat ./.github/workflows/all_python_versions.txt)" >> "$GITHUB_OUTPUT" + - id: load_os_versions + run: echo "os_versions=$(cat ./.github/workflows/all_os_versions.txt)" >> "$GITHUB_OUTPUT" + assess-file-changes: uses: ./.github/workflows/assess-file-changes.yml @@ -29,11 +41,14 @@ jobs: uses: ./.github/workflows/doc-link-checks.yml run-tests: - needs: assess-file-changes + needs: [assess-file-changes, load_python_and_os_versions] if: ${{ needs.assess-file-changes.outputs.SOURCE_CHANGED == 'true' }} uses: ./.github/workflows/testing.yml secrets: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + with: + python-versions: ${{ needs.load_python_and_os_versions.outputs.ALL_PYTHON_VERSIONS }} + os-versions: ${{ needs.load_python_and_os_versions.outputs.ALL_OS_VERSIONS }} run-streaming-tests: needs: assess-file-changes @@ -49,26 +64,11 @@ jobs: secrets: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} - run-dev-gallery: - needs: assess-file-changes - if: ${{ needs.assess-file-changes.outputs.SOURCE_CHANGED == 'true' }} - uses: ./.github/workflows/dev-gallery.yml - test-dandi-latest: needs: assess-file-changes if: ${{ needs.assess-file-changes.outputs.SOURCE_CHANGED == 'true' }} uses: ./.github/workflows/dandi-release.yml - test-dandi-dev: - needs: assess-file-changes - if: ${{ needs.assess-file-changes.outputs.SOURCE_CHANGED == 'true' }} - uses: ./.github/workflows/dandi-dev.yml - - test-dandi-dev-live: - needs: assess-file-changes - if: ${{ needs.assess-file-changes.outputs.SOURCE_CHANGED == 'true' }} - uses: ./.github/workflows/dandi-dev-live-services.yml - check-final-status: name: All tests passing if: always() diff --git a/.github/workflows/dev-dailies.yml b/.github/workflows/dev-dailies.yml new file mode 100644 index 00000000..a2082eeb --- /dev/null +++ b/.github/workflows/dev-dailies.yml @@ -0,0 +1,65 @@ +name: Daily dev workflows + +on: + workflow_dispatch: + schedule: + - cron: "0 14 * * *" # Daily at 10am EST + +jobs: + + test-pynwb-dev: + uses: neurodatawithoutborders/nwbinspector/.github/workflows/pynwb-dev-tests.yml@dev + + test-dandi-dev: + uses: neurodatawithoutborders/nwbinspector/.github/workflows/dandi-dev.yml@dev + + test-dandi-dev-live: + uses: neurodatawithoutborders/nwbinspector/.github/workflows/dandi-dev-live-services.yml@dev + + notify-pynwb-dev-failure: + runs-on: ubuntu-latest + needs: [test-pynwb-dev] + if: ${{ always() && needs.test-pynwb-dev.result == 'failure' }} + steps: + - uses: dawidd6/action-send-mail@v3 + with: + server_address: smtp.gmail.com + server_port: 465 + username: ${{ secrets.MAIL_USERNAME }} + password: ${{ secrets.MAIL_PASSWORD }} + subject: NWB Inspector Daily Failure - pynwb dev branch + to: rly@lbl.gov + from: NWB Inspector + body: "The daily test against the pynwb dev branch failed: please check status at https://github.com/NeurodataWithoutBorders/nwbinspector/actions/workflows/dev-dailies.yml" + + notify-dandi-dev-failure: + runs-on: ubuntu-latest + needs: [test-dandi-dev] + if: ${{ always() && needs.test-dandi-dev.result == 'failure' }} + steps: + - uses: dawidd6/action-send-mail@v3 + with: + server_address: smtp.gmail.com + server_port: 465 + username: ${{ secrets.MAIL_USERNAME }} + password: ${{ secrets.MAIL_PASSWORD }} + subject: NWB Inspector Daily Failure - DANDI dev branch (offline) + to: rly@lbl.gov + from: NWB Inspector + body: "The daily test against the DANDI dev branch (offline) failed: please check status at https://github.com/NeurodataWithoutBorders/nwbinspector/actions/workflows/dev-dailies.yml" + + notify-dandi-dev-live-failure: + runs-on: ubuntu-latest + needs: [test-dandi-dev-live] + if: ${{ always() && needs.test-dandi-dev-live.result == 'failure' }} + steps: + - uses: dawidd6/action-send-mail@v3 + with: + server_address: smtp.gmail.com + server_port: 465 + username: ${{ secrets.MAIL_USERNAME }} + password: ${{ secrets.MAIL_PASSWORD }} + subject: NWB Inspector Daily Failure - DANDI dev branch (live services) + to: rly@lbl.gov + from: NWB Inspector + body: "The daily test against the DANDI dev branch with live services failed: please check status at https://github.com/NeurodataWithoutBorders/nwbinspector/actions/workflows/dev-dailies.yml" diff --git a/.github/workflows/dev-gallery.yml b/.github/workflows/pynwb-dev-tests.yml similarity index 89% rename from .github/workflows/dev-gallery.yml rename to .github/workflows/pynwb-dev-tests.yml index a85085be..3c42d243 100644 --- a/.github/workflows/dev-gallery.yml +++ b/.github/workflows/pynwb-dev-tests.yml @@ -1,4 +1,4 @@ -name: Development Version Gallery +name: Testing against pynwb dev branch on: workflow_call env: @@ -6,7 +6,7 @@ env: jobs: build-and-test: - name: Testing against dev PyNWB version + name: Testing nwbinspector against pynwb dev branch runs-on: ubuntu-latest strategy: fail-fast: false diff --git a/.github/workflows/read-nwbfile-tests.yml b/.github/workflows/read-nwbfile-tests.yml index 1c44d79c..59a24074 100644 --- a/.github/workflows/read-nwbfile-tests.yml +++ b/.github/workflows/read-nwbfile-tests.yml @@ -10,35 +10,34 @@ jobs: name: Streaming tests using ${{ matrix.os }} with ${{ matrix.python-version }} runs-on: ${{ matrix.os }} defaults: - run: - shell: bash -l {0} # necessary for conda + run: + shell: bash -l {0} # necessary for conda activation, including on Windows strategy: fail-fast: false matrix: os: ["ubuntu-latest", "windows-latest"] # TODO: update mac and streaming methods python-version: ["3.10", "3.13"] steps: - - uses: conda-incubator/setup-miniconda@v3 + - uses: actions/checkout@v4 + - run: git fetch --prune --unshallow --tags + + # Use the conda-forge h5py because the PyPI wheel does not bundle the ROS3 driver. + # `activate-environment` + `auto-activate-base: false` are what make the activation + # propagate across steps on Windows; without them, h5py ends up installed into a base + # environment that subsequent `pytest` invocations cannot see. + - name: Set up Conda + uses: conda-incubator/setup-miniconda@v3 with: auto-update-conda: true + activate-environment: ros3 + environment-file: environment-ros3.yml python-version: ${{ matrix.python-version }} channels: conda-forge - - uses: actions/checkout@v4 - - run: git fetch --prune --unshallow --tags - - - name: Install pytest - run: | - pip install pytest - pip install pytest-cov + auto-activate-base: false - name: Install package run: pip install ".[dandi,zarr]" s3fs - - name: Uninstall h5py - run: pip uninstall -y h5py - - name: Install ROS3 - run: conda install -c conda-forge h5py - - name: Run pytest on streaming-based read_nwbfile tests run: pytest -rsx --cov=nwbinspector --cov-report xml:./coverage.xml tests/read_nwbfile_streaming_tests.py diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml index 66f73a31..5a938283 100644 --- a/.github/workflows/testing.yml +++ b/.github/workflows/testing.yml @@ -1,6 +1,15 @@ name: Testing on: workflow_call: + inputs: + python-versions: + description: 'List of Python versions to use in matrix, as JSON string' + required: true + type: string + os-versions: + description: 'List of OS versions to use in matrix, as JSON string' + required: true + type: string secrets: CODECOV_TOKEN: required: true @@ -12,8 +21,8 @@ jobs: strategy: fail-fast: false matrix: - os: ["ubuntu-latest", "macos-latest", "windows-latest"] - python-version: ["3.10", "3.13"] + os: ${{ fromJson(inputs.os-versions) }} + python-version: ${{ fromJson(inputs.python-versions) }} env: TESTING_FILES_FOLDER_PATH: ./204919/testing_files/ steps: diff --git a/CHANGELOG.md b/CHANGELOG.md index f1f9efe0..504786ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,11 @@ ### Improvements * Made `hdmf-zarr` an optional dependency to restore pip-installability when `numcodecs` wheels are unavailable. Install with `pip install nwbinspector[zarr]` to enable Zarr-backed NWB file inspection. The local-file read path now delegates to `pynwb.read_nwb`, which produces a clear install hint when a Zarr file is encountered without the `[zarr]` extra. [#698](https://github.com/NeurodataWithoutBorders/nwbinspector/issues/698) +* Reorganized CI so upstream-dev test failures no longer gate PRs. Dev-branch jobs (`test-pynwb-dev`, `test-dandi-dev`, `test-dandi-dev-live`) moved from `deploy-tests.yml` into a new `dev-dailies.yml` scheduled workflow. Also added per-workflow failure emails, centralized the `testing.yml` Python and OS matrices into shared text files, renamed `dev-gallery.yml` to `pynwb-dev-tests.yml`, and rewrote `read-nwbfile-tests.yml` to use a named conda environment (`environment-ros3.yml`) so the ROS3-enabled h5py from conda-forge activates correctly on Windows runners. [#700](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/700) ### Fixes +* Fixed ROS3 streaming in `read_nwbfile` after h5py began enforcing an explicit `aws_region` for the ROS3 driver. Added a `backend_kwargs` keyword-only parameter to `read_nwbfile` that forwards arbitrary kwargs to the underlying `NWBHDF5IO` constructor on the streaming paths (`fsspec`, `ros3`); ROS3 callers now pass `backend_kwargs={"aws_region": "us-east-1"}`. The generic escape hatch avoids having to surface each new upstream kwarg individually. [#700](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/700) +* Deprecated `read_nwbfile_and_io`; it will be removed after 2026-11-13. Use `read_nwbfile` instead; the IO object is accessible from the returned NWBFile via `nwbfile.get_read_io()`. [#700](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/700) # v0.7.1 (March 26, 2026) diff --git a/environment-ros3.yml b/environment-ros3.yml new file mode 100644 index 00000000..7fa64bb5 --- /dev/null +++ b/environment-ros3.yml @@ -0,0 +1,17 @@ +# Conda environment for ROS3 streaming tests. +# +# h5py from conda-forge ships compiled with the ROS3 virtual file driver enabled, +# which the PyPI wheels still do not (see https://github.com/h5py/h5py/issues/1884). +# This file is consumed by `.github/workflows/read-nwbfile-tests.yml` via +# `setup-miniconda`'s `environment-file` input, following the same pattern PyNWB uses +# to keep ROS3 tests green on Windows. + +name: ros3 +channels: + - conda-forge +dependencies: + - h5py + - pytest + - pytest-cov + - fsspec + - pip diff --git a/src/nwbinspector/_nwb_inspection.py b/src/nwbinspector/_nwb_inspection.py index 552caef6..5a476e2b 100644 --- a/src/nwbinspector/_nwb_inspection.py +++ b/src/nwbinspector/_nwb_inspection.py @@ -17,8 +17,8 @@ from ._registration import Importance, InspectorMessage, available_checks from .tools._read_nwbfile import ( _MissingHdmfZarrError, + _read_nwbfile_and_io, read_nwbfile, - read_nwbfile_and_io, ) from .utils import ( OptionalListOfStrings, @@ -268,7 +268,7 @@ def inspect_nwbfile( io = None try: - in_memory_nwbfile, io = read_nwbfile_and_io(nwbfile_path=nwbfile_path) + in_memory_nwbfile, io = _read_nwbfile_and_io(nwbfile_path=nwbfile_path) if not skip_validate: validation_result = pynwb.validate(path=nwbfile_path) diff --git a/src/nwbinspector/tools/_read_nwbfile.py b/src/nwbinspector/tools/_read_nwbfile.py index be2df201..a4a1d904 100644 --- a/src/nwbinspector/tools/_read_nwbfile.py +++ b/src/nwbinspector/tools/_read_nwbfile.py @@ -58,10 +58,12 @@ def _init_fsspec(path: str) -> "fsspec.AbstractFileSystem": # type: ignore raise ValueError(message) -def read_nwbfile_and_io( +def _read_nwbfile_and_io( nwbfile_path: Union[str, Path], method: Optional[Literal["local", "fsspec", "ros3"]] = None, backend: Optional[Literal["hdf5", "zarr"]] = None, + *, + backend_kwargs: Optional[dict] = None, ) -> tuple[NWBFile, HDMFIO]: """ Read an NWB file using the specified (or auto-detected) method, returning both the file and its IO object. @@ -80,6 +82,13 @@ def read_nwbfile_and_io( backend : "hdf5", "zarr", or None (default) Deprecated. Backend selection is now handled automatically by ``pynwb.read_nwb`` for local files. The argument is accepted but ignored. Will be removed after 12/1/2026. + backend_kwargs : dict, optional + Extra keyword arguments forwarded to the underlying ``NWBHDF5IO`` constructor when streaming + (``method="fsspec"`` or ``method="ros3"``). Useful for arguments the underlying stack requires + but that this helper does not surface directly. The most common case is ROS3, which requires + ``aws_region``: ``backend_kwargs={"aws_region": "us-east-1"}``. Not applicable to local reads + (``pynwb.read_nwb`` is used for those and does not accept extra kwargs); passing + ``backend_kwargs`` with ``method="local"`` raises ``ValueError``. Returns ------- @@ -115,6 +124,11 @@ def read_nwbfile_and_io( raise ValueError( "The ROS3 method was selected, but the URL starts with 's3://'! Please switch to an 'https://' URL." ) + if method == "local" and backend_kwargs is not None: + raise ValueError( + "`backend_kwargs` is only applicable to streaming methods (`fsspec`, `ros3`). " + "Local reads use `pynwb.read_nwb`, which does not accept extra keyword arguments." + ) filterwarnings(action="ignore", message="No cached namespaces found in .*") filterwarnings(action="ignore", message="Ignoring cached namespace .*") @@ -136,6 +150,8 @@ def read_nwbfile_and_io( io_kwargs.update(file=file) else: # ros3 io_kwargs.update(path=nwbfile_path, driver="ros3") + if backend_kwargs: + io_kwargs.update(backend_kwargs) io = NWBHDF5IO(**io_kwargs) return io.read(), io @@ -144,18 +160,49 @@ def read_nwbfile( nwbfile_path: Union[str, Path], method: Optional[Literal["local", "fsspec", "ros3"]] = None, backend: Optional[Literal["hdf5", "zarr"]] = None, + *, + backend_kwargs: Optional[dict] = None, ) -> NWBFile: """ Read an NWB file using the specified (or auto-detected) method. Thin wrapper around ``read_nwbfile_and_io`` that returns only the NWBFile. See ``read_nwbfile_and_io`` for parameter and behavior details, including the - deprecated ``backend`` argument. + deprecated ``backend`` argument and the ``backend_kwargs`` escape hatch for + streaming kwargs. Returns ------- nwbfile : pynwb.NWBFile The in-memory NWBFile object. """ - nwbfile, _ = read_nwbfile_and_io(nwbfile_path=nwbfile_path, method=method, backend=backend) + nwbfile, _ = _read_nwbfile_and_io( + nwbfile_path=nwbfile_path, method=method, backend=backend, backend_kwargs=backend_kwargs + ) return nwbfile + + +def read_nwbfile_and_io( + nwbfile_path: Union[str, Path], + method: Optional[Literal["local", "fsspec", "ros3"]] = None, + backend: Optional[Literal["hdf5", "zarr"]] = None, + *, + backend_kwargs: Optional[dict] = None, +) -> tuple[NWBFile, HDMFIO]: + """ + Deprecated. Use ``read_nwbfile`` (returns just the NWBFile) for the public API. + + This function will be removed after 2026-11-13. The IO object is accessible from the returned + NWBFile via ``nwbfile.get_read_io()`` if a caller needs it, so a separate public helper is no + longer needed. + """ + warn( + "`read_nwbfile_and_io` is deprecated and will be removed after 2026-11-13. " + "Use `read_nwbfile` instead; the IO object is accessible from the returned NWBFile via " + "`nwbfile.get_read_io()`.", + category=DeprecationWarning, + stacklevel=2, + ) + return _read_nwbfile_and_io( + nwbfile_path=nwbfile_path, method=method, backend=backend, backend_kwargs=backend_kwargs + ) diff --git a/tests/read_nwbfile_streaming_tests.py b/tests/read_nwbfile_streaming_tests.py index 9577dab0..1a900a90 100644 --- a/tests/read_nwbfile_streaming_tests.py +++ b/tests/read_nwbfile_streaming_tests.py @@ -1,5 +1,7 @@ """All tests that specifically require streaming to be enabled (i.e., ROS3 version of h5py, fsspec, etc.).""" +import sys + import pytest from nwbinspector.testing import check_hdf5_io_open, check_streaming_tests_enabled @@ -44,10 +46,19 @@ def test_hdf5_fsspec_s3(): @pytest.mark.skipif(not STREAMING_TESTS_ENABLED, reason=DISABLED_STREAMING_TESTS_REASON or "") +@pytest.mark.skipif( + sys.platform == "win32", + reason=( + "The HDF5 ROS3 driver hangs indefinitely on Windows runners (likely a libcurl/TLS " + "issue in the C-level HDF5 ROS3 VFD; pytest-timeout cannot interrupt the C call). " + "The fsspec streaming path is the recommended alternative on Windows." + ), +) def test_hdf5_ros3_https(): nwbfile = read_nwbfile( nwbfile_path=PERSISTENT_READ_NWBFILE_HDF5_EXAMPLE_HTTPS, method="ros3", + backend_kwargs={"aws_region": "us-east-1"}, ) assert check_hdf5_io_open(io=nwbfile.read_io)