Skip to content

Conversation

@AurelienJaquier
Copy link
Collaborator

@AurelienJaquier AurelienJaquier commented Jun 26, 2025

  • new NWB reader for data from Derek Howard (An in vitro whole-cell electrophysiology dataset of human cortical neurons) that can be found at
    10.48324/dandi.000293/0.220708.1652 (human), and
    10.48324/dandi.000292/0.220708.1652 (mouse).
  • also add try/except in SAHP.interpret()

@AurelienJaquier AurelienJaquier self-assigned this Jun 26, 2025
@AurelienJaquier AurelienJaquier requested a review from ilkilic June 26, 2025 13:44
@AurelienJaquier
Copy link
Collaborator Author

I have tested extracting features on both mouse and human data with this new reader and the output looked decent.

@AurelienJaquier
Copy link
Collaborator Author

Here is the code I used to test the new reader.
test.zip
The data to run the test with can be found here and here

Copy link
Collaborator

@darshanmandge darshanmandge left a comment

Choose a reason for hiding this comment

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

Thanks @AurelienJaquier!

Can you add more description to the PR?


def __init__(self, content, target_protocols, in_data, repetition=None):
""" Init
Args:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please add a comment here referring to the dataset + first author, paper name and DOI?

i_unit = current.attrs["unit"].decode('UTF-8')
t_unit = start_time.attrs["unit"].decode('UTF-8')

if (
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like this logic will be only used by one dataset which will use this reader. The _format_nwb_trace is being used by all the NWB readers. @ilkilic do you think we can instead update the data?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree, it would be better to clean the data directly

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But then if someone tries to use BPE directly on the data straight from the source, they won't be able to read them

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think users can use the data downloaded from OBI directly, as it will be cleaner and easier to understand. :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

As we are busy with other tasks, we can use the nwb files in the original format. I will create a task to edit these files to make them simple.

@AurelienJaquier AurelienJaquier changed the title New NWB reader New NWB reader: TRTNWBReader Jul 9, 2025
@AurelienJaquier AurelienJaquier merged commit 90c6c1a into master Jul 10, 2025
4 checks passed
@AurelienJaquier AurelienJaquier deleted the reader branch July 10, 2025 08:40
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.

4 participants