Adding MED format, second submission

[dancrepeau]

Hello,

A month or so ago, my colleague Matt Stead and I pushed a fork with changes to support the MED format (medformat.org). This new pull request is for a version that's a bit more detailed: it finds discontinuities and creates separate Neo segments, and it groups channels together that have the same sampling frequencies. It also reads Note and Neuralynx type annotations, and has two time basis options, zero-based and original time.

Please let me know if there are questions/comments/criticisms! Thank you,
Dan

[JuliaSprenger]

Hi @dancrepeau!
Thanks for rewriting the IO! I think the segment & stream concept is not implemented consistently yet. I left some comments in the code. Tell me if you need more details on this. I think a test file with more than one segment (by design or discontinuous data) would be beneficial here to implement this. Based on the tests you wrote I guess the current test file is a single segment only, right?

[JuliaSprenger]

This is nitpicking, but this change should be one line further down to be in alphabetical order....

[JuliaSprenger]

You are creating one stream per segment and stream template. However, streams are shared between all segments. I think you are generating more streams than required and could just use the stream_templates as streams. Or is this a feature of MED that in each segment you could have different streams and channels?

[JuliaSprenger]

You have implemented methods below to read event data, so you should also create the corresponding event header for those methods to be used.

[JuliaSprenger]

You are implementing a multi-segment IO, so it would make sense provide a (less generic) name for all segments.

[JuliaSprenger]

you are testing basic RawIO functionality here. This should be already covered in other tests in Neo, so I would not duplicate it here.

[JuliaSprenger]

Suggested change

	# There will only be one analogsignals in a MED reading
	# There will only be one analogsignal in a MED reading

[JuliaSprenger]

Depending on your test file the IO should generate more than 1 segment. If there's more segments, could you also check those?

[JuliaSprenger]

You added MED specific annotations to the block. You could confirm that these are correctly annotated on the block level. (in test_read_block)

[dancrepeau]

Hi @JuliaSprenger!

Thank you for reviewing the code. I will go through your comments thoroughly in the coming days. I will comment right away that we are indeed testing against a recording that has discontinuous ranges, specifically "test.medd". Starting on line 76 of test_medrawio.py, we are verifying that it found 3 segments, and there are two unique streams in each segment (6 streams total).

If streams are supposed to be shared across all segments, then I will have to re-work the code. I'm using the convention that a continuous stretch of data is a segment, and within that, a stream is a grouping of channels that have the same sampling frequency. Please correct me if I have those ideas wrong.

My notion of a "template" (which admittedly isn't clear) is just a grouping of channels that have the same sampling frequency. However, there can be lots of discontinuitites within a recording, so I am multiplying the templates by continuous ranges, and assuming those are streams. I am assuming a stream can't have gaps in it.

Thanks again for all the help!
-Dan

[JuliaSprenger]

Hi @dancrepeau,
yes, I think the segment & stream design might need to be adjusted to fit the Neo & RawIO structures.
In general a Neo segment is intended to contain data that shares a common clock. So for many IOs multiple segments are generated when either the recording was paused and restarted or the recording system didn't manage to write all of the continuous data, but lost some samples at a certain time point.

In the RawIO design a stream is independently of segments describing different input channels, characterised e.g. for MED by their different sampling rates. When loading a dataset Neo creates one AnalogSignal per Segment per Stream. This also implies that all channels (recording traces) in a stream have to contain the same number of samples within a segment, to be able to represent the data of that AnalogSignal as a numpy array.

I hope that helps restructuring, let me know if you are still missing some information.

[dancrepeau]

Hi @JuliaSprenger,

Thank you for the clarifications. The terminology here can become very confusing. In MED, segments and continuous sections are independent concepts, as segments are primarily used to break recordings up into smaller files, but can also be used to indicate a new experiment within a recording.

I might suggest some changes to documentation to make these Neo concepts easier for newcomers to digest (and feel free to disregard me here, as I am jus trying to be helpful). The Neo core documentation (https://neo.readthedocs.io/en/stable/core.html) only shows a higher level view; indeed the word "stream" doesn't appear anywhere on this page. I found the file "baserawio.py" to be helpful in understanding the raw io concepts, however I came to some wrong conclusions. For example, the function:

def get_signal_t_start(self, block_index, seg_index, stream_index=None):
"""
Retrieve the t_start of a single section of the channels in a stream.
:param block_index:
:param seg_index:
:param stream_index:
:return: start time of section
"""
refers to a 'section' of a stream, yet 'section' is not a parameter to the function. I believe the correct interpretation is that 'segment' and 'section' are interchangeable (in this context). I came to the conclusion that 'section' is implied by the stream itself, and therefore there was only one start time to a stream.

Again, I appreciate the clarifications!
-Dan