Edit NeuralynxIO to include all valid samples from NCS files (analog signals)

[emelon8]

These edits relate to the treatment of records with <512 valid samples, with the goal of keeping all valid samples. In particular, they fix three cases where valid samples were previously discarded:

1. The last record of each segment, which almost always has <512 valid samples
2. Segments with only one record with 512 valid samples (sandwiched between records with <512 valid samples)
3. Segments consisting of only one record of <512 valid samples

test_neuralynxrawio.py runs OK.

[pep8speaks]

Hello @emelon8! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-03-04 09:58:38 UTC

[JuliaSprenger]

Hi @emelon8,
thanks for proposing this enhancement of the NeuralynxIO. It works fine for me, but I think this would also require an additional test that checks the actual length of the signals read in case of a gap. Currently, the tests are mostly focussing on the existence and identification of the correct numbers of gaps, but not the data samples recovered. Could you e.g. extend

python-neo/neo/test/iotest/test_neuralynxio.py

Line 313 in 8e53515

class TestGaps(CommonNeuralynxIOTest, unittest.TestCase):

to add a test that is checking the number of samples read. Maybe it even makes sense to compare to the plain representation of the data directly for this.

@samuelgarcia @sin-mike What do you think performance wise of this?

[emelon8]

Hi @JuliaSprenger,
The plain representation (and original representation) files seem to have records with <512 valid samples already removed, so it makes it tough to test with that data. If we have good sample data that includes records with <512 valid samples, I think I could add a test for something like

num_samples = 0
num_records = 0
num_samples_from_records_lt_512 = 0
ch = list(recording._sigs_memmap[0].keys())[0]
for seg in block.segments:
    num_samples += len(seg.analogsignals[0])
    num_records += len(seg)
    num_samples_from_records_lt_512 += recording._sigs_memmap[seg.index][ch][-1]['nb_valid']
self.assertEqual(num_samples, num_records*512 + num_samples_from_records_lt_512)

Not sure if that's what you're looking for or not.

[JuliaSprenger]

Hi @emelon8,
thanks for the note, I wasn't aware that the plain data files are lacking samples of incomplete blocks. But I think this is something I can add, if @samuelgarcia and @sin-mike in general agree to the approach in this PR.

[JuliaSprenger]

Hi @emelon8, with #905 being merged into master, the issue of not all valid samples being included might be solved already. Do you mind having a look and tell us if you are happy with the current behaviour?

[emelon8]

Hi @JuliaSprenger, I will take a look at it soon and let you know.

[emelon8]

@JuliaSprenger I wasn't able to import neo:

In [2]: import neo
Traceback (most recent call last):

  File "<ipython-input-2-ee05c7771214>", line 1, in <module>
    import neo

  File "D:\Programs\Anaconda\envs\neotest\lib\site-packages\neo\__init__.py", line 12, in <module>
    from neo.io import *

  File "D:\Programs\Anaconda\envs\neotest\lib\site-packages\neo\io\__init__.py", line 242, in <module>
    from neo.io.axographio import AxographIO

  File "D:\Programs\Anaconda\envs\neotest\lib\site-packages\neo\io\axographio.py", line 9, in <module>
    from neo.rawio.axographrawio import AxographRawIO

  File "D:\Programs\Anaconda\envs\neotest\lib\site-packages\neo\rawio\__init__.py", line 123, in <module>
    from neo.rawio.neuralynxrawio.neuralynxrawio import NeuralynxRawIO

  File "D:\Programs\Anaconda\envs\neotest\lib\site-packages\neo\rawio\neuralynxrawio.py", line 123, in <module>
    from neo.rawio.neuralynxrawio import NeuralynxRawIO

ImportError: cannot import name 'NeuralynxRawIO'

Checking the code, it seems there is nothing to import from neuralynxrawio.py called NeuralynxRawIO anymore. Am I missing something?

[JuliaSprenger]

@emelon8 Sorry, there was still an inconsistency in master in the handling of the new neuralynxrawio files as this IO is now split into multiple files. Have a look at the neo/rawio/neuralynxrawio folder for orientation. If the checks of #917 are succeeding, could you check again?

[emelon8]

@JuliaSprenger The checks didn't succeed in #917. I'm happy to check it once it's ready.

[JuliaSprenger]

Hi @emelon8,
#917 is merged now probably also causing the conflict between your version and master. Could you check again?

[emelon8]

Hi @JuliaSprenger, I finally got around to doing this. It looks like the analog signals from v0.9.0 and the version of v0.8.0 that I edited to keep all valid samples are identical, so I think we're good to go with the current version!

[JuliaSprenger]

@emelon8 Thanks for checking. Since the feature seems to work for you in the current version I will close this PR for now.