spikeinterface icon indicating copy to clipboard operation
spikeinterface copied to clipboard

Correct time units for `BlackrockSortingExtractor`

Open h-mayorquin opened this issue 2 years ago • 2 comments

In #856 I forgot to scale the timestamps of the spikes correctly. This small PR fixes that by scaling the timestamps of neo (which are usually in seconds) to frames.

I also did some small fixes to some of the docstrings in neobaseextractor.

h-mayorquin avatar Aug 03 '22 12:08 h-mayorquin

Ok, so this turned out to be more complicated than I thought.

The problem is that in the test that was failing: https://github.com/SpikeInterface/spikeinterface/runs/7651689921?check_suite_focus=true

The corresponding blackrock file has more than one stream. When that happens, the function from neo that extract _t_start requires the stream_index to be passed as well but that was not available hence the error in CI: https://github.com/SpikeInterface/spikeinterface/blob/4346eff5706408435e8aa8345be22fdd452afa27/spikeinterface/extractors/neoextractors/neobaseextractor.py#L172

Therefore, this PR also adds machinery so that when the function _auto_guess_sampling_frequency(self) extracts the frequency from the corresponding signal, the signal_id (from which we can calculate the signal_index) is also extracted.

h-mayorquin avatar Aug 03 '22 13:08 h-mayorquin

@alejoe91 anything missing here?

h-mayorquin avatar Aug 09 '22 07:08 h-mayorquin

Hi Ramon, This is OK for me. Could you rebase to the actual master ? Be carrfull Alessio some refactoring on neobase extractor.

samuelgarcia avatar Aug 29 '22 15:08 samuelgarcia

@samuelgarcia Done.

h-mayorquin avatar Aug 30 '22 15:08 h-mayorquin