spikeinterface
spikeinterface copied to clipboard
Add 'How SpikeInterface handles time' documentation
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_starton 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_startproperly 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
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.
@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.
Also, the shift_start_time is not in main yet. Is there an open PR for it?
@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.
Hi @JoeZiminski
Would you have time to finalize this one? :)
Hey @alejoe91 sure! I think I will have time over the next 1-2 weeks, feel free to ping me anytime about it