Add timing metadata for SpikeGLXRawIO

[h-mayorquin]

Should close #1607

[h-mayorquin]

I had a discussion with Graham Findlay from the Tononi and Cirelli lab and he suggested to make the smallest t_start the reference point (hence, it would be 0). That seemed like a good idea to me but I wanted to check with you guys @alejoe91 @samuelgarcia.

[samuelgarcia]

Thanks for this. This is really cool. We should have done this from the beginings.
Maybe we could have an option to force the previous behavior (t_start=0 everywhere) in case some people handle event with this in mind for bawkard compatibility.

forcing the very first t_start to be 0 is convinient sometimes (I did for for some readers).
For long term data conversion the original t_start is better ?
Maybe we could have a secondary option for this ...

[h-mayorquin]

Thanks for this. This is really cool. We should have done this from the beginings.
Maybe we could have an option to force the previous behavior (t_start=0 everywhere) in case some people handle event with this in mind for bawkard compatibility.

I believe adding an option as a temporary solution is acceptable, but it should ultimately be removed to minimize maintenance overhead. Here’s my proposal:

Introduce a boolean flag that allows users to enable the old behavior. However, this flag should also trigger a deprecation warning, informing users that the feature will be removed in the next major release.

forcing the very first t_start to be 0 is convinient sometimes (I did for for some readers).
For long term data conversion the original t_start is better ?
Maybe we could have a secondary option for this ...

I think this purpose would be better served by having a general function or boolean option at init that can be used for any reader. This will be easier to maintain, more ergonomic for the user and I think more useful. What do you think?

[alejoe91]

Thanks for updating this @h-mayorquin

I'm not sure we need the boolean option to force t start at 0...as we're making a new minor release, a small API change as this one is ok. Let's document it properly in the release notes @zm711

[samuelgarcia]

Introduce a boolean flag that allows users to enable the old behavior. However, this flag should also trigger a deprecation warning, informing users that the feature will be removed in the next major release.

Lets forget about it, this useless and painfull work.

I think this purpose would be better served by having a general function or boolean option at init that can be used for any reader. This will be easier to maintain, more ergonomic for the user and I think more useful. What do you think?

Yes. this should be a hack in the main class but then we should not reset the first t_start to 0 then.