Skip to content

OpenEphysBinary: do not split ADC streams for OneBox - #1753

Merged
zm711 merged 5 commits into
NeuralEnsemble:masterfrom
alejoe91:fix-onebox-adc
Sep 5, 2025
Merged

OpenEphysBinary: do not split ADC streams for OneBox#1753
zm711 merged 5 commits into
NeuralEnsemble:masterfrom
alejoe91:fix-onebox-adc

Conversation

@alejoe91

@alejoe91 alejoe91 commented Jul 28, 2025

Copy link
Copy Markdown
Contributor

Fixes #1752

@zm711

zm711 commented Jul 28, 2025

Copy link
Copy Markdown
Contributor

@alejoe91, while we wait for the user to test this could you add to the docstring or comment above what the OneBox should look like as far as we know? So it is a three dir system in the main expt dir? Or whatever.

@h-mayorquin

Copy link
Copy Markdown
Contributor

Are we still waiting for data on this one or should I move forward with the review? I think Alessio is on vacation so feel free to reply once you are back.

@zm711

zm711 commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

To me it looks like we have a spikeglx version of data but not an openephys version. So I would prefer

  1. add data of this format so we actually test
  2. we write up a note about data structure

But I think it is fine if you want to review @h-mayorquin. If you have an example at neuroconv you can still test the function without the official test.

@h-mayorquin

Copy link
Copy Markdown
Contributor

Someone shared data with us. Once Alessio is back I can take a look. I just did not understand above if a user already promised they would give us some data.

@alejoe91

Copy link
Copy Markdown
Contributor Author

Someone shared data with us. Once Alessio is back I can take a look. I just did not understand above if a user already promised they would give us some data.

Hi guys, here is the issue with the data (somehow I forgot to link it in the first place!): SpikeInterface/spikeinterface#4083

@alejoe91

Copy link
Copy Markdown
Contributor Author

@zm711 @h-mayorquin can any of you upload the data on GIN? I'm running into the usual git-annex version issue...

The data from @jialeren2024 is here: SpikeInterface/spikeinterface#4083 (comment)

@h-mayorquin

Copy link
Copy Markdown
Contributor

That's great, merged the data. The data that I have is only one box so it will not be useful to test the error. If I read the PR from gin well, the data that @alejoe91 provided does contain both neural and analog one box acquisition data which is what we need.

@zm711

zm711 commented Aug 27, 2025

Copy link
Copy Markdown
Contributor

Seems like the metadata is being misread? Based on the error in the test suite?

@zm711

zm711 commented Aug 28, 2025

Copy link
Copy Markdown
Contributor

@apdavison any word on ebrains? It seems like docs are failing because we are timing out with our connection to download the ebrains dataset?

@zm711 zm711 added this to the 0.14.3 milestone Sep 3, 2025
@zm711

zm711 commented Sep 5, 2025

Copy link
Copy Markdown
Contributor

I'll merge after tests pass!

@zm711
zm711 merged commit 7c899cc into NeuralEnsemble:master Sep 5, 2025
2 of 3 checks passed

@h-mayorquin h-mayorquin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JUst one question that I did not send by mistake yesterday.

self.get_local_path("openephysbinary/v0.6.x_neuropixels_with_sync"), load_sync_channel=True
)
rawio_with_sync.parse_header()
with self.assertWarns(DeprecationWarning):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What deprecation warning is this asserting for?

Could we use https://docs.python.org/3/library/unittest.html#unittest.TestCase.assertWarnsRegex

to match the message

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OpenEphys: wrong ADC split with OneBox

3 participants