Ecephys warnings removal and clean up

[h-mayorquin]

I have been working on ecephys pipeline this week and these are some warnings that made it more difficult for me to find the errors. Besides, I am doing some other minor clean up.

• @pauladkisson, I am removing the stub tests that we discussed before as those are now tested with the mocking interface
• I am deprecating the starting_time of add_electrical_series as we have the time alignment methods now. We did the same in Support digital channels on NIDQ interface and use TimeSeries instead of ElecricalSeries for analog channels #1152

[pauladkisson]

• @pauladkisson, I am removing the stub tests that we discussed before as those are now tested with the mocking interface

Looks good.

• I am deprecating the starting_time of add_electrical_series as we have the time alignment methods now. We did the same in Support digital channels on NIDQ interface and use TimeSeries instead of ElecricalSeries for analog channels #1152

If we are removing the ability to set the starting times directly, we should probably include a way to set the starting_time and rate (currently, the temporal alignment methods require that aligned starting times use full timestamps).

[h-mayorquin]

If we are removing the ability to set the starting times directly, we should probably include a way to set the starting_time and rate (currently, the temporal alignment methods require that aligned starting times use full timestamps).

We have methods for this:

neuroconv/src/neuroconv/datainterfaces/ecephys/baserecordingextractorinterface.py

Lines 185 to 221 in 05423d1

	def set_aligned_starting_time(self, aligned_starting_time: float):
	if self._number_of_segments == 1:
	self.set_aligned_timestamps(aligned_timestamps=self.get_timestamps() + aligned_starting_time)
	else:
	self.set_aligned_segment_timestamps(
	aligned_segment_timestamps=[
	segment_timestamps + aligned_starting_time for segment_timestamps in self.get_timestamps()
	]
	)
	def set_aligned_segment_starting_times(self, aligned_segment_starting_times: list[float]):
	"""
	Align the starting time for each segment in this interface relative to the common session start time.
	Must be in units seconds relative to the common 'session_start_time'.
	Parameters
	----------
	aligned_segment_starting_times : list of floats
	The starting time for each segment of data in this interface.
	"""
	assert len(aligned_segment_starting_times) == self._number_of_segments, (
	f"The length of the starting_times ({len(aligned_segment_starting_times)}) does not match the "
	"number of segments ({self._number_of_segments})!"
	)
	if self._number_of_segments == 1:
	self.set_aligned_starting_time(aligned_starting_time=aligned_segment_starting_times[0])
	else:
	aligned_segment_timestamps = [
	segment_timestamps + aligned_segment_starting_time
	for segment_timestamps, aligned_segment_starting_time in zip(
	self.get_timestamps(), aligned_segment_starting_times
	)
	]
	self.set_aligned_segment_timestamps(aligned_segment_timestamps=aligned_segment_timestamps)

And the function to add ElectricalSeries decides whether to write starting_time + rate or timestamps.

[pauladkisson]

We have methods for this:
...
And the function to add ElectricalSeries decides whether to write starting_time + rate or timestamps.

Ok, so to set the starting time, you would call set_aligned_starting_time, which would

1. use recording_extractor.get_times() to automatically construct a full timestamps vector from starting_time and rate
2. add aligned_starting_time to every timestamp in that vector
3. when the add_electrical_series_to_nwbfile sees these aligned timestamps, it would calculate that they are regular, and extract the starting time and rate to put in the nwb file.

I guess this works, it's a little convoluted/inefficient, but simple for the user, which is more important...

I think this is a case (temporal alignment in general really) where it would be really nice to have a how-to in the docs to make it easier to use. But...that's definitely outside the scope of this PR and would require some discussion.

[h-mayorquin]

Correct.

Yes, it is inefficient as of now. I will make some improvements from SpikeInterface. The advantage of this is just to have this code in only one place instead of having two ways of doing the same spread across the codebase. Plus, it is easier to explain, what happens if you pass both start_time and then use the time alignment methods? Which one has priority? are they additive or overwrite each other?

I think this is a case (temporal alignment in general really) where it would be really nice to have a how-to in the docs to make it easier to use. But...that's definitely outside the scope of this PR and would require some discussion.

Yes, that would be nice.

[pauladkisson]

Looks good!

[codecov]

Codecov Report

Attention: Patch coverage is 81.81818% with 2 lines in your changes missing coverage. Please review.

Project coverage is 90.75%. Comparing base (96dfdff) to head (90ccdc1).
Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
...c/neuroconv/tools/spikeinterface/spikeinterface.py	50.00%	1 Missing ⚠️
...c/neuroconv/tools/testing/data_interface_mixins.py	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1158      +/-   ##
==========================================
+ Coverage   90.69%   90.75%   +0.05%     
==========================================
  Files         129      129              
  Lines        8189     8285      +96     
==========================================
+ Hits         7427     7519      +92     
- Misses        762      766       +4     

Flag	Coverage Δ
unittests	90.75% <81.81%> (+0.05%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...rfaces/ecephys/blackrock/blackrockdatainterface.py	95.00% <ø> (ø)
...rfaces/ecephys/neuralynx/neuralynxdatainterface.py	86.07% <100.00%> (ø)
src/neuroconv/tools/testing/mock_interfaces.py	99.18% <100.00%> (+0.03%)	⬆️
...c/neuroconv/tools/spikeinterface/spikeinterface.py	90.97% <50.00%> (-0.15%)	⬇️
...c/neuroconv/tools/testing/data_interface_mixins.py	95.65% <80.00%> (+0.09%)	⬆️

... and 3 files with indirect coverage changes