define new EphysDataSet interface in detail

We have a new, source-independent interface for accessing ephys data, but a couple of questions remain (background). In particular, we should answer:

for each method defined on EphysDataSet if the return type does not fully describe the returned data (e.g. because it is a dataframe), we must document the returned data in more detail
for each method defined on EphysDataSet we must decide if the method should be part of our public interface.
for each attribute which makes up part of the public interface of EphysDataSet, we must ensure that this attribute is clearly defined, rather than implicitly set in a method.
For methods and attributes of the old EphysDataSet but not defined on the new one, we need to have a clearly state if, how, and when this functionality will be supported.

EDIT: I've done some investigation (see comment). Here is what I think remains to be done for this issue:

make sure dataset.EphysDataSet has all required attributes at the correct level of privacy (Fully typed and documented EphysDataSet #366)
make sure each such attribute has typing (Fully typed and documented EphysDataSet #366)
make sure each such attribute is documented (Fully typed and documented EphysDataSet #366)
make sure EphysDataSet methods are not doing EphysDataInterface's work (Fully typed and documented EphysDataSet #366)
tests for EphysDataSet (weirdly, an allensdk issue: test for EphysDataSetClass AllenSDK#1433)

GH-361 fix typo in data interface filename

GH-361 start specifying abstract base for ephys dataset

#361 checkpoint

How is EphysDataSet constructed and used?

Construction:

not constructed

EphysDataSet is frequently used as a class object, particularly to access its constants.

data_set_features
- EphysDataSet.SHORT_SQUARE
- EphysDataSet.RAMP
- EphysDataSet.LONG_SQUARE
sweep_props
- EphysDataSet.SWEEP_NUMBER
- EphysDataSet.STIMULUS_UNITS
- EphysDataSet.STIMULUS_AMPLITUDE
- EphysDataSet.STIMULUS_CODE
- EphysDataSet.STIMULUS_NAME

by helper function

Old version (data_set_utils) defines:

def create_data_set(
    sweep_info=None, 
    nwb_file=None, 
    ontology=None, 
    api_sweeps=True, 
    h5_file=None, 
    validate_stim=True
):    
    """Create an appropriate EphysDataSet derived class for the given nwb_file

    Parameters
    ----------
    nwb_file: str file name

    Returns
    -------
    EphysDataSet derived object
    """

This is used in:

bin.plot_ephys_nwb : create_data_set(nwb_file=nwb_file,validate_stim=False,ontology=ont)
bin.run_feature_extraction: create_data_set(sweep_info=sweep_info, nwb_file=input_nwb_file, ontology=ont, api_sweeps=False)
bin.run_sweep_extraction: create_data_set(nwb_file=input_nwb_file, h5_file=input_h5_file, ontology=ont)

New version (dataset.create) defines:

def create_ephys_data_set(nwb_file: str,
                          sweep_info: Optional[Dict[str,Any]],
                          ontology_file: Optional[str]) -> EphysDataSet:

    """
    Create an ephys data set with the appropriate nwbdata reader class

    Parameters
    ----------
    nwb_file
    sweep_info
    ontology_file

    Returns
    -------

    EphysDataSet

    """

by `AibsDataSet`

A bunch of examples directly construct an AibsDataSet from an NWB1 file.

e.g from spike_train_features:

dataset = AibsDataSet(sweep_info=sweep_info, nwb_file=nwb_file)

the script_utils also do this:

data_set = AibsDataSet(nwb_file=nwb_path, h5_file=h5_path, ontology=ontology)

by `HBGDataSet` or `MIESDataSet`

nowhere in IPFX

Usage

Of course these attributes might be used outside of IPFX.

old `EphysDataSet`

organizing this by attribute. Old EphysDataSet defines publicly:

sweep_table (used commonly)
ontology (used by e.g. the qc extractor)
validate_stim (used only internally)
nwb_data (used internally)
build_sweep_table(self, sweep_info=None) (used internally)
add_clamp_mode(self, sweep_info) (used internally)
filtered_sweep_table(self, clamp_mode=None, stimuli=None, stimuli_exclude=None) (used commonly ... for some reason :/)
get_sweep_number(self, stimuli, clamp_mode=None) (used by e.g. the qc feature extractor)
get_sweep_record(self, sweep_number) (used internally)
sweep(self, sweep_number) (used commonly)
sweep_set(self, sweep_numbers) (used commonly)
aligned_sweeps(self, sweep_numbers, stim_onset_delta) (never implemented, thus never used)
extract_sweep_stim_info(self) (used internally)
get_recording_date(self) (used occasionally)
get_sweep_data(self, sweep_number) (used internally)
get_clamp_mode(self,stimulus_number) (used internally)
get_stimulus_code(self,stimulus_number) (used internally)
get_stimulus_name(self, stim_code) (used internally)

AibsDataSet(EphysDataSet) defines:

extract_sweep_stim_info(self) (used internally)
get_stimulus_code(self, sweep_num) (used internally)
get_stimulus_code_ext(self, stim_code,sweep_num) (used internally)
get_stimulus_units(self, sweep_num) (used internally)
get_clamp_mode(self, sweep_num) (used internally)
get_recording_date(self) (used occasionally)

HBGDataSet(EphysDataSet) defines:

extract_sweep_record(self, sweep_num: int) -> Dict[str, Any] (used internally)
extract_sweep_stim_info(self) (used internally)
get_stimulus_units(self, sweep_num) (used internally)
get_clamp_mode(self, sweep_num) (used internally)
get_stimulus_code(self, sweep_num) (used internally)
get_stimulus_code_ext(self, sweep_num) (used internally)
get_recording_date(self) (used occasionally)

MiesDataSet(HBGDataSet) defines:

notebook (used internally)
get_stimulus_code_ext(self, sweep_num) (used internally)
extract_sweep_record(self, sweep_num: int) -> Dict[str, Any] (used internally)

new `EphysDataSet`

has the following public attributes:

data
ontology
extract_sweep_info(self) -> Dict[str,Any]
build_sweep_table(self, sweep_info=None)
sweep_table
filtered_sweep_table(self, clamp_mode=None, stimuli=None, stimuli_exclude=None)
get_sweep_number(self, stimuli, clamp_mode=None)
voltage_current(self, sweep_data, clamp_mode) -> Tuple[np.arrray,np.array]
sweep(self, sweep_number)
sweep_set(self, sweep_numbers)
drop_trailing_zeros_in_response(sweep_data) (a staticmethod)

The design specifies

build_sweep_table(self, sweep_info: Optional[Dict[str,Any]] = None)
filtered_sweep_table(self, clamp_mode: Optional[str]=None,stimuli: Optional[List[str]]=None,stimuli_exclude=None,) -> pd.DataFrame
get_sweep_number(self, stimuli: List[str], clamp_mode: str=None) -> int
get_sweep_record(self, sweep_number: int) -> Dict[str,Any]
sweep(self, sweep_number:int)-> Sweep
sweep_set(self, sweep_numbers: List[int]) -> SweepSet
get_sweep_data(self, sweep_number: int) -> Dict
get_stimulus_name(self, stim_code: str) -> st

.data is an EphysDataInterface subclass. These have:

get_sweep_data(self, sweep_number: int) -> Dict[str,Any]
get_sweep_record(self, sweep_number: int) -> Dict[str,Any]
get_sweep_attrs(self, sweep_number) -> Dict[str,Any]
get_sweep_number(self, sweep_name:str)-> int
get_stim_code(self, sweep_number: int) -> str
get_stimulus_name(self, stim_code: str, validate: Optional[bool] = True) -> str
get_stim_code_ext(self, sweep_number: int)-> str
get_session_start_time(self) -> str
get_recording_date(self) -> str
get_stimulus_unit(self, sweep_number: int) -> str
get_real_sweep_number(self, sweep_name:str, assumed_sweep_number: Optional[int]=None) -> int
get_starting_time(self, data_set_name: str) -> str
build_sweep_map(self)
drop_reacquired_sweeps(self)
get_sweep_map(self, sweep_number)
get_acquisition_groups(self) -> List[str]
get_stimulus_groups(self) ->List[str]
get_stimulus_name(self, stim_code)

In the design, these have:

get_sweep_data(self, sweep_number)
get_sweep_number(self, sweep_name)
get_stim_code(self, sweep_number)
get_spike_times(self, sweep_number)
get_stimulus_code(self, sweep_number) (twice?)
get_session_start_time(self)
get_recording_date(self)
get_stimulus_unit(self, sweep_number)
get_unit_name(stim_attrs) (staticmethod)
get_long_unit_name(unit) (staticmethod)
validate_SI_unit(unit) (staticmethod)
get_real_sweep_number(self, sweep_name, assumed_sweep_number=None)
get_starting_time(self, data_set_name)
get_sweep_attrs(self, sweep_number)
build_sweep_map(self)
drop_reacquired_sweeps(self)
get_sweep_names(self)
get_sweep_map(self, sweep_number)
get_acquisition_groups(self)
get_stimulus_groups(self)
get_metadata(self, sweep_number: Optional[int] = None) -> Dict[str,Any]

(I've excluded a couple of adder methods, which are out of scope)

Our goals

We are dropping NWB1 support. This means:

a major version increment to 1.0.0
our examples, which download NWB1s from warehouse, will need to be updated

We should also try to

separate the NWB (or other source) reading capabilities of EphysDataInterface from data access role of EphysDataSet
avoid breaking changes unless they're really necessary
- give people some warning before we make the necessary ones
clearly delineate our public interface by hiding methods we don't intend users to call

Plan

Changes to dev version

Here is my proposal for changes to the current dev version of EphysDataSet.

We should hide or otherwise dissolve the following attributes. No user should access these:

data :: Client code should not use the reader directly
extract_sweep_info(self) -> Dict[str,Any] :: only internally useful
build_sweep_table(self, sweep_info=None) :: only internally useful
voltage_current(self, sweep_data, clamp_mode) -> Tuple[np.arrray,np.array] :: seems like an internal-only utility (?). Also not clear why it's an instance method
drop_trailing_zeros_in_response(sweep_data) :: also seems like an internal utility

There are some attributes of the old EphysDataSet which we ought to restore:

get_recording_date(self)
get_sweep_data(self, sweep_number)
get_clamp_mode(self,stimulus_number)
get_stimulus_code(self,stimulus_number)
get_stimulus_code_ext(self, stimulus_number)
get_stimulus_units(self, sweep_num)
get_stimulus_name(self, stim_code)

Additionally, we ought to move data-source-specific work (such as rescaling data) from EphysDataSet to the interface classes

To manage the 1.0.0 release, we should

in current ipfx, do a 0.2.0 release deprecating current direct construction of EphysDataSet subclasses
- rationale, these are going to break in the future (current versions argue filepaths, support nwb1). We should warn people
in the dev branch, update EphysDataSet to match this interface
- make sure all methods are present
- finish docstrings
- finish typing
in the dev branch, remove the old dataset code + tests
in the dev branch, update the examples to use the new interface
- We neeed a place to host these files. Easiest is to just commit (they are ~10s of mb, could use lfs if this is too big).
in the dev branch, update the lims utilities to use the new interface
update EphysDataSet so that it no longer does e.g. unit scaling
tests for EphysDataSet

#361 remove get_acquisition_groups from base ephysdatainterface

#361 put stimulus back

#361 fix imports on ephysdataset

#361 start testing ephysdataset

#361 remove outdated get_sweep_number, get_real_sweep_number (numbers…

#361 complete ipfx ephysdataset tests

#361 fix import of stimulus

#361 update docstrings on ephsdataset; remove get_stimulus_groups fro…

#361 require opt in for labnotebook test that tries to download an nw…

due to this work being subsumed into #366 and AllenInstitute/AllenSDK#1433, closing

Navigation Menu

Repository navigation

define new EphysDataSet interface in detail #361

Construction:

not constructed

by helper function

by `AibsDataSet`

by `HBGDataSet` or `MIESDataSet`

Usage

old `EphysDataSet`

new `EphysDataSet`

Our goals

Plan

Changes to dev version

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

define new EphysDataSet interface in detail #361

Description

Activity

NileGraddis commented on Mar 17, 2020

Construction:

not constructed

by helper function

by AibsDataSet

by HBGDataSet or MIESDataSet

Usage

old EphysDataSet

new EphysDataSet

Our goals

Plan

Changes to dev version

NileGraddis commented on Mar 20, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

by `AibsDataSet`

by `HBGDataSet` or `MIESDataSet`

old `EphysDataSet`

new `EphysDataSet`