Skip to content

Conversation

tdincer
Copy link
Contributor

@tdincer tdincer commented Feb 2, 2023

Based on the "One file per channel type format" section of the official intan manual (page 11): https://intantech.com/files/Intan_RHS2000_data_file_formats.pdf

Task list:

  • file path (channel) - output connection needs to be done. If pattern returns more than 1 file, the outputs should be clearly associated with the file name. This is optional. Current version works fine if the pattern is a full file name (e.g "amp-B-000.dat").
  • Ideally read_qstring and read_header functions should be imported from intan. We need to decide on how to operate the intan. I guess the decision will be made with Alessio.
  • Do basic reading
  • Add docstrings.

For the completeness of the discussion:

  • The official example notebook does not work for Alex's data.

@kabilar kabilar marked this pull request as draft February 3, 2023 09:14
@kabilar
Copy link
Collaborator

kabilar commented Feb 3, 2023

Thanks @tdincer. Drafted this pull request since we wouldn't want to add hardcoded paths to this package.

@kabilar kabilar changed the title working example Add working example of Intan loader Feb 3, 2023
@tdincer
Copy link
Contributor Author

tdincer commented Feb 3, 2023

@kabilar This PR is to a new branch. It's meant to keep track of the changes. Iterations should be done forking this PR. Not by copy pasting the content.

@tdincer tdincer marked this pull request as ready for review February 3, 2023 11:57
@tdincer
Copy link
Contributor Author

tdincer commented Feb 3, 2023

@JaerongA and @kabilar I cleaned the code. Let's iterate from here addressing the issues discussed in slack.

@tdincer tdincer changed the title Add working example of Intan loader Intan loader Feb 3, 2023
@tdincer tdincer requested review from JaerongA and kabilar February 3, 2023 12:13
@tdincer
Copy link
Contributor Author

tdincer commented Feb 3, 2023

@JaerongA What do we need to return to make this loader useful for array-ephys?

@tdincer tdincer requested a review from ttngu207 February 3, 2023 15:55
@JaerongA
Copy link
Collaborator

JaerongA commented Feb 3, 2023

@JaerongA What do we need to return to make this loader useful for array-ephys?

The original intan code used to ouput everything (including all the parameters) into a single dictionary format. We might needs parameter values as well. To be completely sure of what's really needed, it'd be worth ingesting some sample data in the element-array-ephys pipeline.

@JaerongA JaerongA self-requested a review February 3, 2023 17:06
return header


def load_rhs(folder: str, file_expr: str):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
def load_rhs(folder: str, file_expr: str):
def load_rhs(folder: str):

I don't think regex need to be specified here as an argument. This is supposed to fetch whatever that's inside the data folder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It adds flexibility to not read everything. Say the files are amp-B-000.dat ... to amp-B-015.dat, amp-D-000.dat ... to amp-D-015.dat and board-DIGITAL-IN-01.dat.

To read all channels (A and B) in the working directory:
load_rhs(".", "amp*")

To read only the B channels in the working directory:
load_rhs(".", "amp-B*")

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see. Then we can keep that and set the default to e.g.,*.dat (read everything) and allow a user to specify if needed.

Copy link
Collaborator

@JaerongA JaerongA left a comment

Choose a reason for hiding this comment

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

@tdincer. Looks like this only reads from amp.dat files. Some sessions have digital input and stimulus files.

@kabilar
Copy link
Collaborator

kabilar commented Feb 3, 2023

@kabilar This PR is to a new branch. It's meant to keep track of the changes. Iterations should be done forking this PR. Not by copy pasting the content.

Thanks @tdincer. Whoops, misread the pull request. You are right.

@JaerongA JaerongA merged commit 1e1abae into datajoint:intan Feb 3, 2023
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.

3 participants