Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cbin_file_path #3524

Merged
merged 4 commits into from
Nov 19, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 20 additions & 10 deletions src/spikeinterface/extractors/cbin_ibl.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

from pathlib import Path
import warnings
import numpy as np

import probeinterface
Expand Down Expand Up @@ -30,8 +31,10 @@ class CompressedBinaryIblExtractor(BaseRecording):
stream_name : {"ap", "lp"}, default: "ap".
Whether to load AP or LFP band, one
of "ap" or "lp".
cbin_file : str or None, default None
cbin_file_path : str or None, default None
alejoe91 marked this conversation as resolved.
Show resolved Hide resolved
The cbin file of the recording. If None, searches in `folder_path` for file.
cbin_file : str or None, default None
(deprecated) The cbin file of the recording. If None, searches in `folder_path` for file.

Returns
-------
Expand All @@ -41,14 +44,21 @@ class CompressedBinaryIblExtractor(BaseRecording):

installation_mesg = "To use the CompressedBinaryIblExtractor, install mtscomp: \n\n pip install mtscomp\n\n"

def __init__(self, folder_path=None, load_sync_channel=False, stream_name="ap", cbin_file=None):
def __init__(
self, folder_path=None, load_sync_channel=False, stream_name="ap", cbin_file_path=None, cbin_file=None
):
from neo.rawio.spikeglxrawio import read_meta_file

try:
import mtscomp
except ImportError:
raise ImportError(self.installation_mesg)
if cbin_file is None:
if cbin_file is not None:
warnings.warn(
"The `cbin_file` argument is deprecated, please use `cbin_file_path` instead", DeprecationWarning
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we need a stack level for this? I forget?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch

alejoe91 marked this conversation as resolved.
Show resolved Hide resolved
)
cbin_file_path = cbin_file
if cbin_file_path is None:
folder_path = Path(folder_path)
# check bands
assert stream_name in ["ap", "lp"], "stream_name must be one of: 'ap', 'lp'"
Expand All @@ -60,17 +70,17 @@ def __init__(self, folder_path=None, load_sync_channel=False, stream_name="ap",
assert (
len(curr_cbin_files) == 1
), f"There should only be one `*.cbin` file in the folder, but {print(curr_cbin_files)} have been found"
cbin_file = curr_cbin_files[0]
cbin_file_path = curr_cbin_files[0]
else:
cbin_file = Path(cbin_file)
folder_path = cbin_file.parent
cbin_file_path = Path(cbin_file_path)
folder_path = cbin_file_path.parent

ch_file = cbin_file.with_suffix(".ch")
meta_file = cbin_file.with_suffix(".meta")
ch_file = cbin_file_path.with_suffix(".ch")
meta_file = cbin_file_path.with_suffix(".meta")

# reader
cbuffer = mtscomp.Reader()
cbuffer.open(cbin_file, ch_file)
cbuffer.open(cbin_file_path, ch_file)

# meta data
meta = read_meta_file(meta_file)
Expand Down Expand Up @@ -119,7 +129,7 @@ def __init__(self, folder_path=None, load_sync_channel=False, stream_name="ap",
self._kwargs = {
"folder_path": str(Path(folder_path).resolve()),
"load_sync_channel": load_sync_channel,
"cbin_file": str(Path(cbin_file).resolve()),
"cbin_file_path": str(Path(cbin_file_path).resolve()),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more question because I forget the json mechanics. Is this breaking for trying to read the extractor or will this just work?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No this is back compatible since we kept both arguments

}


Expand Down