spikeinterface icon indicating copy to clipboard operation
spikeinterface copied to clipboard

Add 'How SpikeInterface handles time' documentation

Open JoeZiminski opened this issue 1 year ago • 1 comments

In-progress draft of a page detailing how time is represented in SpikeInterface. closes #2627. Once a first draft is finished will require a lot of revision I'm sure as I'm not certain on a lot of this.

Worth discussion is where this goes, at present is a How-to but it is a little tenuous. Maybe it can be reworded to 'How to handle time on SpikeInterface recordings' with a bit more code added.

(An aside, please excuse if there already solutions to this I have missed). As the documentation grows API changes will frequently break existing documentation. One solution is to add a template to all PR that has a tick-box stating that all relevant documentation must be reviewed and updated before merge. This is good practice anyway but is error-prone. There seem to be some existing solutions to testing code snippets within documentation: pylit[https://pypi.org/project/pylit/] sphinx. Actually I see from searching @h-mayorquin might be able to advise on this!

EDIT: some things to do before continuing

  • currently I don't think it is possible to manually set t_start on a recording object. Instead it is only loaded from certain extractors. As far as I can tell. It would be cool to make this accessible, this would then make the tutorial make more sense.
  • currently generate_ground_truth_recording has no t_start properly and it is not possible to set times that then relate to the sorting. Would be useful to add this before continuing here. discussed in #3071

JoeZiminski avatar Jun 24 '24 17:06 JoeZiminski

You can find PR templates here:

https://github.com/NeurodataWithoutBorders/pynwb/tree/dev/.github/PULL_REQUEST_TEMPLATE

But let's discuss this as a separate issue. I personally don't really like them tbh but I think they are mostly good especially for the issues.

Regarding the where this should go, my two cents: I read your draft and I think this is clearly a tutorial. You are trying to teach users how the library does something, not how to solve a specific problem.

h-mayorquin avatar Jun 24 '24 23:06 h-mayorquin

@JoeZiminski I was going through this again and it looks really good! :)

We could just add a reference to the get_duration/get_total_duration and the recently added get_start_time/get_end_time as well.

alejoe91 avatar Jan 27 '25 08:01 alejoe91

Also, the shift_start_time is not in main yet. Is there an open PR for it?

alejoe91 avatar Jan 27 '25 08:01 alejoe91

@alejoe91

There is shift_times that shifts either t_start or the times:

https://github.com/SpikeInterface/spikeinterface/pull/3509/files

That is, is independent of implementation details.

h-mayorquin avatar Jan 28 '25 22:01 h-mayorquin

Hi @JoeZiminski

Would you have time to finalize this one? :)

alejoe91 avatar Jan 29 '25 16:01 alejoe91

Hey @alejoe91 sure! I think I will have time over the next 1-2 weeks, feel free to ping me anytime about it

JoeZiminski avatar Jan 30 '25 10:01 JoeZiminski