[neuralynx] exploit metadata available from neo.neuralynxrawio

[JuliaSprenger]

Exploit metadata available from neo NeuralynxIO file headers and annotations. This PR is an update of catalystneuro/nwb-conversion-tools#596
Contributes to #43

Motivation

Recent versions of Neo provide access to header metadata from recording files. There is no need to re-read and interpret the headers twice.

Checklist

• Have you thoroughly read our Developer Guide?
• Have you ensured the PR description clearly describes the problem and solutions?
• Have you checked to ensure that there aren't other open or previously closed Pull Requests for the same change?
• If this PR fixes an issue, is the first line of the PR description fix #XXX where XXX is the issue number?
• Did you update the CHANGLOG.md file on your branch to describe your changes?

[bendichter]

@JuliaSprenger this looks great! Can you add tests please?

[JuliaSprenger]

This PR will be simplified based on #174

[JuliaSprenger]

The coverage seems to decrease as code lines intended for the next neo version are not covered and test files are not downloaded as they are cached.

[h-mayorquin]

Thanks for implementing the suggestions above @JuliaSprenger .

[h-mayorquin]

@JuliaSprenger
The tests that is failing is because some of the filtered columns is not in the following types: a list, np.ndarray, string or Real object (from the numbers library). Can you show here the column types of the filtered related column, I wonder what is the column that is failing the type check.

[JuliaSprenger]

All properties were numpy arrays, but it seems an object (e.g. datetime.datetime objects) or bool dtype is not accepted by nwb. So I added some lines to convert these to a string representation.

[h-mayorquin]

Oh, yes, this is OK, thanks for changing the types, we should accept bools and I will fix that into another PR. One last thing, can you remove the annotation out of the initialization, as I mentioned in our discussion we don't really propagate those.

[codecov]

Codecov Report

Merging #170 (94c91ce) into main (81113f0) will decrease coverage by 0.87%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##             main     #170      +/-   ##
==========================================
- Coverage   91.63%   90.76%   -0.88%     
==========================================
  Files          65       65              
  Lines        3409     3423      +14     
==========================================
- Hits         3124     3107      -17     
- Misses        285      316      +31     

Flag	Coverage Δ
unittests	90.76% <85.71%> (-0.88%)	⬇️

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

Impacted Files	Coverage Δ
...rfaces/ecephys/neuralynx/neuralynxdatainterface.py	88.23% <85.71%> (-8.07%)	⬇️
src/neuroconv/tools/data_transfers.py	56.33% <0.00%> (-35.22%)	⬇️

[h-mayorquin]

Thanks for all the hard work.

[bendichter]

@JuliaSprenger It looks like there are int and bool values here there encoded as strings. Is there a reason for that?

[JuliaSprenger]

For export of these properties we need to convert bool values to strings, as we otherwise get an error in nwb (see #185 first checkpoint).

[bendichter]

the electrodes table should be able to handle ints and bools with no problem. What's the error in NWB?

[CodyCBakerPhD]

It's not an error with NWB, it's an error with channel properties that are bool which are then used in our add_electrodes function (see #185) which doesn't have a default type set for that class, which is something needed when writing multiple recording objects (not all of which have that same boolean channel property)

[CodyCBakerPhD]

The float and int values here shouldn't have to be strings though

[bendichter]

OK, well we can change the bools to uint8 that's almost as good