Confusing default behavior for `time_slice` docstring

[kushaangupta]

The way that this docstring is phrased makes it appear that the argument can be omitted but instead it should say that if the start time is None

spikeinterface/src/spikeinterface/core/baserecording.py

Line 774 in 03c818c

The start time in seconds. If not provided it is set to 0.

[zm711]

That's a great point! Want to make a PR fixing it to say something like:

The start time in seconds. If set to None the start time will be taken as 0.

And the same for end_time?

[kushaangupta]

@zm711 Thanks for the quick response. Just opened one

[h-mayorquin]

I think that we should set defaults to None instead. Having to input None is strange now that I think about it.

[zm711]

We are bouncing around but just to have the point in both places. Then this would mean

new_recording = old_recording.time_slice()
new_recording == old_recording

Doesn't that seem like a weird silent mechanism? Vs raising an error saying you have to make a decision by providing the arguments. I honestly think the end user should have to give at least one of the two arguments (similar to numpy slicing right where we can either do :x or x:, but I forget the slicing discussion so maybe we don't want to seem too be too similar to that indexing. What do you think about that instead?

my vote would be (in descending preference)

1. give either beginning or end: required (infer the other)
2. give both: required
3. give neither: infer automatically

[h-mayorquin]

I vote for 3. This will be easier for both users and us the maintainers (less assertion codes to check)

Doesn't that seem like a weird silent mechanism?

Yes, it is a strange edge case but it has zero consequences.

I would like to hear a meaningful confusion that we are avoiding by increasing the complexity in usage.

Practically beats purity and all of that.

I honestly think the end user should have to give at least one of the two arguments (similar to numpy slicing right where we can either do :x or x:,

You can do array[:] and that gets the array.

[zm711]

I would like to hear a meaningful confusion that we are avoiding by increasing the complexity in usage.

Practically beats purity and all of that.

Two points that I don't think rise to your threshold:

1. confusion about what the arguments are because it silently works (users should read the docstrings and see the typing)
2. provenance and unsliced recording will appear as timeslicedrecording even though it is not actually sliced in anyway (maybe you care about this? maybe we do?)

You can do array[:] and that gets the array.

Sorry I was using the mechanism as an argument for providing the edges. But even in this case you are explicitly saying you want the whole array (the equivalent of the user explicitly saying time_slice(None, None) for our logic.