Recorders with non voltage (non standard) channels and slicing channels

[h-mayorquin]

In #761 we move the warnings of non standard units to get_traces. We achieved this by introducing an extra attribute:

spikeinterface/spikeinterface/core/baserecording.py

Lines 109 to 116 in 486bc67

	if return_scaled:
	if hasattr(self, "NeoRawIOClass"):
	if self.has_non_standard_units:
	message = (
	f'This extractor based on neo.{self.NeoRawIOClass} has channels with units not in (V, mV, uV)'
	)
	warnings.warn(message)

However, as far as I know this attribute (self.has_non_standards_units) is not copied when we slice the channels with recorder.slice_channel which indicates problems. I think that what would happen right now is that the attribute NeoRawIOClass is not copied either so the sliced recorder with non voltage units will never raise the warning.

Maybe we should handle this with annotations as you already have the machinery in those methods to copy that?

Context: #758, #761 and NeuralEnsemble/python-neo#1133

[alejoe91]

@h-mayorquin you can extend the copy_metadata function: https://github.com/SpikeInterface/spikeinterface/blob/master/spikeinterface/core/base.py#L227

[samuelgarcia]

I think I would go for an annotation like this '__has_non_standards_units__' and not an attribute.

[h-mayorquin]

@alejoe91 we should probably move this warning to get_traces to the neobase recording withing the library.

[samuelgarcia]

what if you slice ? I think in base it is OK.

[alejoe91]

Hi suggest adding this to the NeoBaseRecordingExtractor:

def get_traces(self,
               segment_index: Union[int, None] = None,
               start_frame: Union[int, None] = None,
               end_frame: Union[int, None] = None,
               channel_ids: Union[Iterable, None] = None,
               order: Union[str, None] = None,
               return_scaled=False,
               cast_unsigned=False
               ):
    if return_scaled:
            if hasattr(self, "NeoRawIOClass"):
                if self.has_non_standard_units:
                    message = ( 
                    f'This extractor based on neo.{self.NeoRawIOClass} has channels with units not in (V, mV, uV)'
                    )
                    warnings.warn(message)
    return super().get_traces(segment_index, start_frame, end_frame, channek_ids, order, return_scaled, cast_unsigned)

This would have the same effect without "polluting" the base with NEO related stuff (the BaseRecording doesn't even know neo exists!!!)

[samuelgarcia]

ok for me.

[samuelgarcia]

except the cascade of if that could be done in one line with and

[alejoe91]

except the cascade of if that could be done in one line with and

????

[h-mayorquin]

All right, so we should:

1. Do this in NeoBaseRecordingExtractor as @alejoe91 points out, and
2. Do this with attributes so the copying machinery of slicing works.

Please say if you agree with both and I will take care of it,

[alejoe91]

@h-mayorquin please go ahead!