Skip to content

Commit

Permalink
Deprecate ros3 as a streaming method (#2662)
Browse files Browse the repository at this point in the history
* deprecate ros3

* remove open blas

* return ibl to streaming extractors

* add upper bound scipy

---------

Co-authored-by: Alessio Buccino <[email protected]>
  • Loading branch information
h-mayorquin and alejoe91 authored Apr 4, 2024
1 parent 969bef9 commit ea83a86
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 21 deletions.
11 changes: 1 addition & 10 deletions .github/workflows/streaming-extractor-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ jobs:
update-conda: true
python-version: ${{ matrix.python-version }}
conda-channels: conda-forge
- run: git fetch --prune --unshallow --tags
- name: Install openblas
run: sudo apt install libopenblas-dev # Necessary for ROS3 support
- name: Get changed files
id: changed-files
uses: tj-actions/changed-files@v41
Expand All @@ -47,12 +44,6 @@ jobs:
- name: Install package and streaming extractor dependencies
if: ${{ steps.modules-changed.outputs.STREAMING_CHANGED == 'true' }}
run: pip install -e .[test_core,streaming_extractors]
# Temporary disabled because of complicated error with path
# - name: Install h5py with ROS3 support and test it works
# run: |
# pip uninstall -y h5py
# conda install -c conda-forge "h5py>=3.2"
# python -c "import h5py; assert 'ros3' in h5py.registered_drivers(), f'ros3 suppport not available, failed to install'"
- name: run tests
if: steps.modules-changed.outputs.STREAMING_CHANGED == 'true'
run: pytest -m "streaming_extractors and not ros3_test" -vv -ra
run: pytest -m "streaming_extractors" -vv -ra
4 changes: 3 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ extractors = [
streaming_extractors = [
"ONE-api>=2.7.0", # alf sorter and streaming IBL
"ibllib>=2.32.5", # streaming IBL
"scipy<1.13", # ibl has a dependency on scipy but it does not have an upper bound
# Remove this once https://github.com/int-brain-lab/ibllib/issues/753
# Following dependencies are for streaming with nwb files
"fsspec",
"aiohttp",
Expand Down Expand Up @@ -113,6 +115,7 @@ qualitymetrics = [

test_core = [
"pytest",
"pytest-dependency",
"psutil",

# for github test : probeinterface and neo from master
Expand Down Expand Up @@ -200,7 +203,6 @@ markers = [
"widgets",
"sortingcomponents",
"streaming_extractors: extractors that require streaming such as ross and fsspec",
"ros3_test"
]
filterwarnings =[
'ignore:.*distutils Version classes are deprecated.*:DeprecationWarning',
Expand Down
32 changes: 24 additions & 8 deletions src/spikeinterface/extractors/nwbextractors.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def read_file_from_backend(
*,
file_path: str | Path | None,
file: BinaryIO | None = None,
stream_mode: Literal["ffspec", "ros3", "remfile"] | None = None,
stream_mode: Literal["ffspec", "remfile"] | None = None,
cache: bool = False,
stream_cache_path: str | Path | None = None,
storage_options: dict | None = None,
Expand Down Expand Up @@ -98,7 +98,7 @@ def read_nwbfile(
backend: Literal["hdf5", "zarr"],
file_path: str | Path | None,
file: BinaryIO | None = None,
stream_mode: Literal["ffspec", "ros3", "remfile", "zarr"] | None = None,
stream_mode: Literal["ffspec", "remfile", "zarr"] | None = None,
cache: bool = False,
stream_cache_path: str | Path | None = None,
storage_options: dict | None = None,
Expand All @@ -112,7 +112,7 @@ def read_nwbfile(
The path to the NWB file. Either provide this or file.
file : file-like object or None
The file-like object to read from. Either provide this or file_path.
stream_mode : "fsspec" | "ros3" | "remfile" | None, default: None
stream_mode : "fsspec" | "remfile" | None, default: None
The streaming mode to use. If None it assumes the file is on the local disk.
cache: bool, default: False
If True, the file is cached in the file passed to stream_cache_path
Expand All @@ -132,12 +132,12 @@ def read_nwbfile(
Notes
-----
This function can stream data from the "fsspec", "ros3" and "rem" protocols.
This function can stream data from the "fsspec", and "rem" protocols.
Examples
--------
>>> nwbfile = read_nwbfile(file_path="data.nwb", backend="hdf5", stream_mode="ros3")
>>> nwbfile = read_nwbfile(file_path="data.nwb", backend="hdf5", stream_mode="fsspec")
"""

if file_path is not None and file is not None:
Expand Down Expand Up @@ -428,7 +428,7 @@ class NwbRecordingExtractor(BaseRecording):
samples_for_rate_estimation: int, default: 1000
The number of timestamp samples used for estimating the sampling rate. This is relevant
when the 'rate' attribute is not available in the ElectricalSeries.
stream_mode : "fsspec" | "ros3" | "remfile" | "zarr" | None, default: None
stream_mode : "fsspec" | "remfile" | "zarr" | None, default: None
Determines the streaming mode for reading the file. Use this for optimized reading from
different sources, such as local disk or remote servers.
load_channel_properties: bool, default: True
Expand Down Expand Up @@ -485,7 +485,7 @@ def __init__(
electrical_series_name: str | None = None, # deprecated
load_time_vector: bool = False,
samples_for_rate_estimation: int = 1_000,
stream_mode: Optional[Literal["fsspec", "ros3", "remfile", "zarr"]] = None,
stream_mode: Optional[Literal["fsspec", "remfile", "zarr"]] = None,
stream_cache_path: str | Path | None = None,
electrical_series_path: str | None = None,
load_channel_properties: bool = True,
Expand All @@ -495,6 +495,14 @@ def __init__(
storage_options: dict | None = None,
use_pynwb: bool = False,
):

if stream_mode == "ros3":
warnings.warn(
"The 'ros3' stream_mode is deprecated and will be removed in version 0.103.0. "
"Use 'fsspec' stream_mode instead.",
DeprecationWarning,
)

if file_path is not None and file is not None:
raise ValueError("Provide either file_path or file, not both")
if file_path is None and file is None:
Expand Down Expand Up @@ -910,7 +918,7 @@ class NwbSortingExtractor(BaseSorting):
samples_for_rate_estimation: int, default: 100000
The number of timestamp samples to use to estimate the rate.
Used if "rate" is not specified in the ElectricalSeries.
stream_mode : "fsspec" | "ros3" | "remfile" | "zarr" | None, default: None
stream_mode : "fsspec" | "remfile" | "zarr" | None, default: None
The streaming mode to use. If None it assumes the file is on the local disk.
stream_cache_path: str or Path or None, default: None
Local path for caching. If None it uses the system temporary directory.
Expand Down Expand Up @@ -964,6 +972,14 @@ def __init__(
storage_options: dict | None = None,
use_pynwb: bool = False,
):

if stream_mode == "ros3":
warnings.warn(
"The 'ros3' stream_mode is deprecated and will be removed in version 0.103.0. "
"Use 'fsspec' stream_mode instead.",
DeprecationWarning,
)

self.stream_mode = stream_mode
self.stream_cache_path = stream_cache_path
self.electrical_series_path = electrical_series_path
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
from spikeinterface.extractors import NwbRecordingExtractor, NwbSortingExtractor


@pytest.mark.ros3_test
@pytest.mark.streaming_extractors
@pytest.mark.skipif("ros3" not in h5py.registered_drivers(), reason="ROS3 driver not installed")
def test_recording_s3_nwb_ros3(tmp_path):
Expand Down Expand Up @@ -155,7 +154,6 @@ def test_recording_s3_nwb_remfile_file_like(tmp_path):
check_recordings_equal(rec, rec2)


@pytest.mark.ros3_test
@pytest.mark.streaming_extractors
@pytest.mark.skipif("ros3" not in h5py.registered_drivers(), reason="ROS3 driver not installed")
def test_sorting_s3_nwb_ros3(tmp_path):
Expand Down

0 comments on commit ea83a86

Please sign in to comment.