search

catalystneuro / roiextractors

Python-based module for extracting from, converting between, and handling optical imaging data from several file formats. Inspired by SpikeInterface.
https://roiextractors.readthedocs.io/en/latest/index.html
BSD 3-Clause "New" or "Revised" License
12 stars 7 forks source link

Add `get_times()` method to imaging classes #379

Open h-mayorquin opened 2 weeks ago

h-mayorquin commented 2 weeks ago

We have set_times:

https://github.com/catalystneuro/roiextractors/blob/e8eb2cf5ff0c11341b419ef800d8bffd397409b0/src/roiextractors/imagingextractor.py#L189-L212

In neuroconv we use this construct to get the times: https://github.com/catalystneuro/neuroconv/blob/419ab11104ff9e67d63f6400aacb3e12081c90d1/src/neuroconv/tools/roiextractors/roiextractors.py#L534-L535

Maybe we should have a method that does this for us like in spikeinterface?

https://github.com/SpikeInterface/spikeinterface/blob/bfe9fb649b58c48d2a04949061154f75f8e08b1f/src/spikeinterface/core/baserecording.py#L429-L450

Thoughts?

pauladkisson commented 2 weeks ago

We already have frame_to_time, which basically performs this function. But maybe it should be renamed? And/or maybe it should provide all the times by default?

h-mayorquin commented 2 weeks ago

Let's not have function with the same name than the spikeinterface one but with a different signature and behavior. The second option would work for me though.

I would prefer to unify the extractor API and just follow spikeinterface here. get_times can be a wrapper around frame_to_time if code duplication is your concern.