Skip to content

stub options #670

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.

Sign up for GitHub

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

Draft
luiztauffer wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

stub options #670

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fb46bc7
stub options
luiztauffer Nov 30, 2023
c5ecbcd
Merge branch 'main' into stub-test-options
luiztauffer Nov 30, 2023
bfb2005
Merge branch 'main' into stub-test-options
luiztauffer Dec 1, 2023
2ad8aaa
Merge branch 'main' into stub-test-options
luiztauffer Dec 6, 2023
8ab40d8
Merge branch 'main' into stub-test-options
luiztauffer Jan 9, 2024
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
14 changes: 10 additions & 4 deletions src/neuroconv/datainterfaces/ecephys/baserecordingextractorinterface.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,17 +271,20 @@ def align_by_interpolation(
else:
raise NotImplementedError("Multi-segment support for aligning by interpolation has not been added yet.")

def subset_recording(self, stub_test: bool = False):
def subset_recording(self, stub_test_size: int = None):
Copy link
Contributor

@CodyCBakerPhD CodyCBakerPhD Nov 30, 2023

Choose a reason for hiding this comment

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

Breaks back-compatability; please add deprecation cycle to stub_test

"""
Subset a recording extractor according to stub and channel subset options.

Parameters
----------
stub_test : bool, default: False
stub_test_size : int, default: None
"""
from spikeinterface.core.segmentutils import ConcatenateSegmentRecording

max_frames = 100
if stub_test_size is None:
max_frames = 100
else:
max_frames = stub_test_size

recording_extractor = self.recording_extractor
number_of_segments = recording_extractor.get_num_segments()
Expand All @@ -300,6 +303,7 @@ def add_to_nwbfile(
nwbfile: NWBFile,
metadata: Optional[dict] = None,
stub_test: bool = False,
stub_test_size: int = 100,
Comment on lines 305 to +306
Copy link
Contributor

@CodyCBakerPhD CodyCBakerPhD Nov 30, 2023

Choose a reason for hiding this comment

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

I would also recommend calling this stub_frames or anything else to indicate it is not stubbing channels along the way (I know it's in the docstring, but if it's in the variable name that's even clearer)

Copy link
Collaborator

@h-mayorquin h-mayorquin Dec 1, 2023

Choose a reason for hiding this comment

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

We use this already, don't we?

neuroconv/src/neuroconv/datainterfaces/ophys/baseimagingextractorinterface.py

Lines 109 to 110 in 74689fd

stub_frames: int = 100,
):

starting_time: Optional[float] = None,
write_as: Literal["raw", "lfp", "processed"] = "raw",
write_electrical_series: bool = True,
Expand All @@ -325,6 +329,8 @@ def add_to_nwbfile(
Sets the starting time of the ElectricalSeries to a manually set value.
stub_test : bool, default: False
If True, will truncate the data to run the conversion faster and take up less memory.
stub_test_size : int, default: 100
The number of frames to keep if stub_test is True.
write_as : {'raw', 'lfp', 'processed'}
write_electrical_series : bool, default: True
Electrical series are written in acquisition. If False, only device, electrode_groups,
Expand Down Expand Up @@ -362,7 +368,7 @@ def add_to_nwbfile(
from ...tools.spikeinterface import add_recording

if stub_test or self.subset_channels is not None:
recording = self.subset_recording(stub_test=stub_test)
recording = self.subset_recording(stub_test_size=stub_test_size)
Comment on lines -365 to +371
Copy link
Contributor

@CodyCBakerPhD CodyCBakerPhD Nov 30, 2023

Choose a reason for hiding this comment

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

Same comment above; this would break all current scripts relying on stub_test as the syntax for testing conversions. Not to mention the GUIDE

else:
recording = self.recording_extractor

Expand Down