IntanRawIO: Update support of `rhs` files #1457

zm711 · 2024-04-06T15:23:15Z

This will be adding support for RHX rhs files with a separate header for rhs files. We haven't officially update RHS support since RHS 1.0 and we are on 3.x so this may take a bit of work. We will add tests files soon so that we can properly test this.

TODOS

~~Add support for supply and aux channels in RHS~~ Turns out that there isn't any from newest docs!
Add support for one-file-per-signal
Add support for one-file-per-channel
Add test files for new formats
Add access to digital channels like we did for rhd
Try to understand the stim channels to see if we need to make updates
Add channel_number_dict for one-file-per-signal
Add analog-out in the one-file-per-channel-dict function

Neuroconv provenance catalystneuro/neuroconv#789

neo/rawio/intanrawio.py

zm711

I accidentally removed these booleans during one of my commits

neo/rawio/intanrawio.py

zm711

Missing one more file test folder the rhs--fps

neo/test/iotest/test_intanio.py

neo/test/rawiotest/test_intanrawio.py

zm711

Bad-copy paste of the test files on my part. I think this should fix the testing portion at last.

neo/test/iotest/test_intanio.py

neo/test/rawiotest/test_intanrawio.py

neo/rawio/intanrawio.py

h-mayorquin

This in general looks good for me. We are replicating the structure that we did for #1402. I would go further for the aproach of segregating the two options (as in the suggestion of having two create_one_file_per_signal_dict insead of one) but these are already in the realm of weak personal preference.

The tests are failing, let's see what's going on and I will read it again but I think this mostly works.

neo/rawio/intanrawio.py

h-mayorquin · 2024-05-03T02:12:54Z

neo/rawio/intanrawio.py

@@ -822,13 +914,37 @@ def read_rhd(filename, file_format: str):
 ]


-def create_one_file_per_signal_dict(dirname):
-    """Function for One File Per Signal Type"""
+def create_one_file_per_signal_dict(dirname, rhs: bool = False):


Personal preference so your milleagea might vary, I rather have two functions here instead of mixing the two of them with keywords. I prefer code duplication rather than cyclomatic complexity and given that these are almost dictionaries / configurations I don't think that duplication is a cognitive burden.

I think that might be fair for this. Mainly because we update the rhd a bit more often than the rhs. So splitting so that we can update the one without breaking the other is a good idea. I would say let's keep this in draft and I will work on that.

neo/rawio/intanrawio.py

zm711 · 2024-05-03T10:46:16Z

The test is failing due to timestamp issue.... That means the test file could be corrupt. Not sure how since this is a test file generated from Intan's simulator. Since it is a binary file I can just read it myself and see if I can figure out what is going on. I think for me what I would say is:

let me fully separate out rhs and rhd for this. I agree with you that trying to squish them together might be trying to hack too much. Some duplication is probably a good idea so we can adapt and adjust one without needing to worry about the other.
Figure out why timestamps are messed up (they really shouldn't be).

h-mayorquin · 2024-05-03T16:02:12Z

The test is failing due to timestamp issue.... That means the test file could be corrupt. Not sure how since this is a test file generated from Intan's simulator. Since it is a binary file I can just read it myself and see if I can figure out what is going on. I think for me what I would say is:

let me fully separate out rhs and rhd for this. I agree with you that trying to squish them together might be trying to hack too much. Some duplication is probably a good idea so we can adapt and adjust one without needing to worry about the other.

Figure out why timestamps are messed up (they really shouldn't be).

Sounds good, let me know if you need to upload something again.

zm711

The number of times I've mistyped these test files is outrageous....

neo/test/iotest/test_intanio.py

neo/test/rawiotest/test_intanrawio.py

neo/rawio/intanrawio.py

zm711 · 2024-05-06T10:53:29Z

@samuelgarcia @alejoe91

This is actually ready for review now. Tests are passing (barring the retest happening right now which should pass). With this PR we now support all formats of Intan (rhd and rhs, traditional + headerless binary formats). If you think it would be beneficial for this release feel free to review now. Otherwise we can wait. @h-mayorquin said he would do another read-over later. So we can either merge sooner and then fix up or wait and tidy more now.

I would prefer squashing this one. I had some fights with typos in the paths to the test files, so there are a lot of junk commits.

alejoe91 · 2024-05-06T11:16:30Z

Thanks @zm711

This looks good to me! @h-mayorquin if you want to take another look, otherwise we can squash and merge and include this in the release :)

h-mayorquin · 2024-05-06T18:26:05Z

I took a second look, it looks good to me.

zm711 · 2024-05-06T19:14:52Z

Thanks @h-mayorquin!

initial fixes

034f96e

zm711 added enhancement IO labels Apr 6, 2024

zm711 self-assigned this Apr 6, 2024

zm711 added 3 commits April 6, 2024 11:29

oops

35ba654

add in dtypes and channel numbers for rhs

5b55401

Merge branch 'master' into intan-rhs

e11b9eb

zm711 commented Apr 8, 2024

View reviewed changes

neo/rawio/intanrawio.py Outdated Show resolved Hide resolved

neo/rawio/intanrawio.py Show resolved Hide resolved

neo/rawio/intanrawio.py Show resolved Hide resolved

add test files in tests more fixes

d55f3aa

zm711 commented Apr 9, 2024

View reviewed changes

neo/rawio/intanrawio.py Outdated Show resolved Hide resolved

zm711 commented Apr 10, 2024

View reviewed changes

neo/rawio/intanrawio.py Outdated Show resolved Hide resolved

neo/rawio/intanrawio.py Outdated Show resolved Hide resolved

zm711 commented May 2, 2024

View reviewed changes

neo/test/iotest/test_intanio.py Outdated Show resolved Hide resolved

Fix naming of test files

f82eb61

zm711 commented May 3, 2024

View reviewed changes

neo/test/rawiotest/test_intanrawio.py Outdated Show resolved Hide resolved

Update neo/test/rawiotest/test_intanrawio.py

fbb2ebf

zm711 commented May 3, 2024

View reviewed changes

neo/test/iotest/test_intanio.py Outdated Show resolved Hide resolved

neo/test/rawiotest/test_intanrawio.py Outdated Show resolved Hide resolved

Fix pointers to test files

13ab6c5

zm711 commented May 3, 2024

View reviewed changes

neo/rawio/intanrawio.py Outdated Show resolved Hide resolved

oops

e01f638

zm711 commented May 3, 2024

View reviewed changes

neo/rawio/intanrawio.py Outdated Show resolved Hide resolved

neo/rawio/intanrawio.py Outdated Show resolved Hide resolved

neo/rawio/intanrawio.py Outdated Show resolved Hide resolved

dtype fixes

78700d7

h-mayorquin reviewed May 3, 2024

View reviewed changes

zm711 added this to the 0.14.0 milestone May 3, 2024

zm711 added 3 commits May 3, 2024 17:52

Merge branch 'NeuralEnsemble:master' into intan-rhs

6435265

split rhs and rhd + fix timestamps

fa49d39

put streams in correct order

10bf3c6

zm711 commented May 3, 2024

View reviewed changes

neo/test/iotest/test_intanio.py Outdated Show resolved Hide resolved

neo/test/rawiotest/test_intanrawio.py Outdated Show resolved Hide resolved

fix test file naming.

cb869c3

zm711 commented May 4, 2024

View reviewed changes

neo/rawio/intanrawio.py Outdated Show resolved Hide resolved

neo/rawio/intanrawio.py Outdated Show resolved Hide resolved

typo fix

ae41690

zm711 commented May 4, 2024

View reviewed changes

neo/rawio/intanrawio.py Outdated Show resolved Hide resolved

zm711 added 2 commits May 4, 2024 18:31

make sure stream_ids are sorted

987e07b

fix one-file-per-signal

6c5743f

zm711 marked this pull request as ready for review May 6, 2024 10:49

alejoe91 mentioned this pull request May 6, 2024

Release notes for 0.13.1, plus an update to the list of authors. #1469

Merged

zm711 modified the milestones: 0.14.0, 0.13.1 May 6, 2024

apdavison merged commit 4e57cc1 into NeuralEnsemble:master May 7, 2024
3 checks passed

zm711 deleted the intan-rhs branch May 7, 2024 09:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IntanRawIO: Update support of `rhs` files #1457

IntanRawIO: Update support of `rhs` files #1457

zm711 commented Apr 6, 2024 •

edited

Loading

zm711 left a comment

zm711 left a comment

zm711 left a comment •

edited

Loading

h-mayorquin left a comment

h-mayorquin May 3, 2024

zm711 May 3, 2024

zm711 commented May 3, 2024 •

edited

Loading

h-mayorquin commented May 3, 2024

zm711 left a comment

zm711 commented May 6, 2024

alejoe91 commented May 6, 2024

h-mayorquin commented May 6, 2024

zm711 commented May 6, 2024

IntanRawIO: Update support of rhs files #1457

IntanRawIO: Update support of rhs files #1457

Conversation

zm711 commented Apr 6, 2024 • edited Loading

zm711 left a comment

Choose a reason for hiding this comment

zm711 left a comment

Choose a reason for hiding this comment

zm711 left a comment • edited Loading

Choose a reason for hiding this comment

h-mayorquin left a comment

Choose a reason for hiding this comment

h-mayorquin May 3, 2024

Choose a reason for hiding this comment

zm711 May 3, 2024

Choose a reason for hiding this comment

zm711 commented May 3, 2024 • edited Loading

h-mayorquin commented May 3, 2024

zm711 left a comment

Choose a reason for hiding this comment

zm711 commented May 6, 2024

alejoe91 commented May 6, 2024

h-mayorquin commented May 6, 2024

zm711 commented May 6, 2024

IntanRawIO: Update support of `rhs` files #1457

IntanRawIO: Update support of `rhs` files #1457

zm711 commented Apr 6, 2024 •

edited

Loading

zm711 left a comment •

edited

Loading

zm711 commented May 3, 2024 •

edited

Loading