search

flatironinstitute / spikeforest_old

SpikeForest -- spike sorting analysis for website
Apache License 2.0
16 stars 8 forks source link

force SFSortingExtractors to have self._sampling_frequency #66

Open alejoe91 opened 5 years ago

alejoe91 commented 5 years ago

With this ST update (https://github.com/SpikeInterface/spiketoolkit/pull/177) the comparison functions take a delta_time rather than delta_frames.

This requires the SortingExtractor to have a self._samapling_frequency which is returned by the get_sampling_frequency functions. All of the updated extractors implement this automatically.

The only issue is the MdaSortingExtractor, because it doesn't save the sampling frequency.

@magland @jamesjun maybe we could add this info to the .mda header?

alejoe91 commented 5 years ago

something like this https://github.com/SpikeInterface/spikeextractors/pull/196 would fix the MdaSortingExtractor. In general I think that it's always good to have the sampling frequency information for sorting extractors

magland commented 5 years ago

I agree it is always good to have sampling freq info in the sorting extractors. However, it is too late to build it into the firings.mda format. Instead we should explicitly pass samplerate=.... into the constructor of the MdaSortingExtractor. The info can be obtained from the accompanying recording extractor.

magland commented 5 years ago

Also, I don't like the following in SortingExtractor.py:

def get_sampling_frequency(self):
        return self._sampling_frequency

The mechanism for a sorting extractor to specify the sampling frequency should not be to set a private variable. Rather it should be to call a set_sampling_frequency(...) --- or to overload the default behavior.

alejoe91 commented 5 years ago

Ok! I will fix that and apply the changes to spikesorters so that when the output is returned it loads the sampling frequency.

We have a set_sampling_frequency, and that it's the mechanism that the user should use to load sampling frequency info. Internally set_sampling_frequency changes the self._sampling_frequency

magland commented 5 years ago

Okay makes sense. We should just make sure that nothing else sets ._sampling_frequency (always use the set_sampling_frequency method).

On Mon, Jul 1, 2019 at 9:14 AM Alessio Buccino notifications@github.com wrote:

Ok! I will fix that and apply the changes to spikesorters so that when the output is returned it loads the sampling frequency.

We have a set_sampling_frequency, and that it's the mechanism that the user should use to load sampling frequency info. Internally set_sampling_frequency changes the self._sampling_frequency

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/flatironinstitute/spikeforest/issues/66?email_source=notifications&email_token=AA4CIQFR4DQDURWVE7DBFOTP5H7KDA5CNFSM4H4MO572YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY6CSMY#issuecomment-507259187, or mute the thread https://github.com/notifications/unsubscribe-auth/AA4CIQB5IL5LAVOQZWJZRSTP5H7KDANCNFSM4H4MO57Q .