plot_timeseries time axis is incorrect if requesting time longer than the recording.

[JoeZiminski]

This is only a small problem and would be fixed with #1989, but just logging it here. If plot_timeseries is called with a time_range that is larger than that available in the file, it will be plot with an incorrect time axis.

For example, calling plot_timeseries with time_range=50 on a 1-second recording has the time axis range [0, 50]. It would be better to throw an error or only show the real time values.

[zm711]

@JoeZiminski , out of curiosity which version are you using. plot_timeseries doesn't exist any more. It has been changed to plot_traces :)

https://github.com/SpikeInterface/spikeinterface/blob/78761bc811cfc424a6a20a67dfbaaeafb5a2f176/src/spikeinterface/widgets/traces.py#L119C1-L123

That is the code that we should fix with something like this maybe:

fs = rec0.get_sampling_frequency()
if time_range is None:
    time_range = (0, 1.0)
time_range = np.array(time_range)
assert time_range[1] < rec0.get_duration(segment_index=segment_index), (f'max time range should be '
                                                          f'{rec0.get_duration(segment_index=segment_index}')

Although in the actual traces retrieval portion of the code it just silently clips to the correct length. Thus the only real problem is that xlim that is being fed into the graph. So the other option would be something like this (which then also warns them that we are clipping the data retrieval as well):

fs = rec0.get_sampling_frequency()
if time_range is None:
    time_range = (0, 1.0)
time_range = np.array(time_range)
if time_range[1] > rec0.get_duration(segment_index=segment_index):
    warn.warnings("you've enter a time value out of range, range will be clipped accordingly")
    time_range[1] = rec0.get_duration(segment_index=segment_index)

I think I would just prefer to be warned that I messed up and let the plotting fix itself rather than break my flow with an assert, but maybe that's the wrong approach...

[JoeZiminski]

Thanks @zm711, I switched to 0.99.1 for the test and saw the deprecation warnings after I posted this 😅

I think either is okay, indeed it is nice to not have the flow broken and most of the benefits of the assert are also provided by the warning. If as a user you are operating under wrong assumptions (e.g. thing a recording is a different length than it is) it may indicate a deeper problem (e.g. you are working on a completely different subject than you thought you were) which could be useful to have the machine shout at you. I think though on balance as it is not a critical function and you can imagine it been annoying having to input the exact end time or just under to see the full recording. So I agree the clip / warning sounds like the best approach!

[zm711]

Cool I'll put in a PR for this as long as @alejoe91 and @samuelgarcia are okay with the warning/clip approach and do not prefer raising an error?

[alejoe91]

Yeo sounds good for me!

[JoeZiminski]

Thanks @zm711!