spikeinterface icon indicating copy to clipboard operation
spikeinterface copied to clipboard

Simplification of signatures wrt noise_levels

Open yger opened this issue 1 year ago • 3 comments

After discussing with @alejoe91, we realized that it would be good to remove noise_levels from the function signatures, since now noise_levels can be saved as attributes of the recording. By simple modification of the base object, noise_levels are saved and pickled (for parallel processing), and thus they can be automatically infered from the recording if already computed.

yger avatar Jan 02 '24 13:01 yger

I think this makes sense. If the noise is a canonical property of the recorder as @alejoe91 once told me, then we should treat it as one. That also has the benefits of simplifying the signatures.

I htink you should add deprecations to the removal of the signatures though and not remove them sharply as you are doing it here.

h-mayorquin avatar Jan 03 '24 11:01 h-mayorquin

I am not sure to like this. And maybe I am almost sure to dislike. I think that the probe a a canical property of recording but noise level is more complicated because it depend on parameters (how many chunk, std vs rms vs mad). So in that case it is better to keep noise level explicit averywhere. Lets discuss this with a call.

samuelgarcia avatar Jan 09 '24 09:01 samuelgarcia

Current, noise levels are already treated as a "special" property, since they are cached and used as a private attribute. So to me, this is making the whole API more consistent. But we can discuss that.

yger avatar Jan 09 '24 09:01 yger