plot_isi_distribution throwing a spike train error

[fruce-ki]

Hello!

I only see one mention of that getter method in the docs, but no doc for the method itself. It appears indeed to not be a method of the Sorting Analyzer.

pui = siwg.plot_isi_distribution(analyzer, unit_ids = u, window_ms=100.0, bin_ms=1.0,
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/SOFTWARE/spikeinterface/src/spikeinterface/widgets/isi_distribution.py", line 38, in __init__
    BaseWidget.__init__(self, plot_data, backend=backend, **backend_kwargs)
  File "/SOFTWARE/spikeinterface/src/spikeinterface/widgets/base.py", line 87, in __init__
    self.do_plot()
  File "/SOFTWARE/spikeinterface/src/spikeinterface/widgets/base.py", line 108, in do_plot
    func(self.data_plot, **self.backend_kwargs)
  File "/SOFTWARE/spikeinterface/src/spikeinterface/widgets/isi_distribution.py", line 61, in plot_matplotlib
    times_ms = sorting.get_unit_spike_train(unit_id=unit_id, segment_index=segment_index) / fs * 1000.0
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'SortingAnalyzer' object has no attribute 'get_unit_spike_train'

[chrishalcrow]

Hi @fruce-ki - plot_isi_distribution currently only accepts a sorting object (https://spikeinterface.readthedocs.io/en/stable/api.html#spikeinterface.widgets.plot_isi_distribution), which you can access from the analyzer. So you can run something like

pui = siwg.plot_isi_distribution(analyzer.sorting, ...)

Many of the other plotting methods accept an analyzer or a sorting. So we could commit to allowing all widgets to accept an analyzer. @alejoe91 @samuelgarcia opinions?

[alejoe91]

Hi @fruce-ki - plot_isi_distribution currently only accepts a sorting object (https://spikeinterface.readthedocs.io/en/stable/api.html#spikeinterface.widgets.plot_isi_distribution), which you can access from the analyzer. So you can run something like

pui = siwg.plot_isi_distribution(analyzer.sorting, ...)
Many of the other plotting methods accept an analyzer or a sorting. So we could commit to allowing all widgets to accept an analyzer. @alejoe91 @samuelgarcia opinions?

Yes we should! especially because isi_histograms is an available extension!

[fruce-ki]

@chrishalcrow Ooops, I guess I need to clean my glasses more often. I completely missed that the first argument isn't an analyzer like the other plots accept.

But I'm happy that an improvement is coming out of my stupid post.

[chrishalcrow]

haha - we should celebrate the many contributions to humanity that began as stupid posts :)
(PS: and not stupid at all! The rest of our API would naturally suggest that the function should accept an analyzer!)

[zm711]

Historically I believe the divide was more related to the fact that the waveform extractor required extracting waveforms which was slow and long (whereas the analyzer grants more flexibility in how to analyze data) so the goal was to have the plotting functions that only required sorting to accept that so that people would have a clear divide between the steps requiring the slower computations and those that could be done quickly. As Chris said with the analyzer api this divide is now not important to make, but the functions still need to updated for this fact. So it actually super beneficial to have users point out where we still need to update functions for the analyzer work flow :)