Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature]: add trials to metadata #997

Closed
2 tasks done
bendichter opened this issue Aug 11, 2024 · 5 comments · May be fixed by #999
Closed
2 tasks done

[Feature]: add trials to metadata #997

bendichter opened this issue Aug 11, 2024 · 5 comments · May be fixed by #999

Comments

@bendichter
Copy link
Contributor

What would you like to see added to NeuroConv?

I would like to be able to add a "trials" field to metadata and have it automatically write a trials table to the NWB file.

Is your feature request related to a problem?

I saw the following exchange on the new chatbot:

bot:

Hi! What can I help you with?

user:

How can I convert an Open Ephys folder to NWB?

bot:

To convert an Open Ephys folder to NWB, you can use the OpenEphysRecordingInterface from the NeuroConv library. Here are the steps to do this:
[correct response]

user:

How can I add trials?

bot:

To add trials to your NWB file during the conversion process, you can extend the metadata dictionary to include trial information. Here’s an example of how you can do this:

Define your trials data: You need to create a list of dictionaries where each dictionary represents a trial and includes the necessary information such as start time, stop time, and any other relevant metadata.

Update the metadata dictionary with the trials information: You can add the trials information to the metadata dictionary under the "trials" key.

Run the conversion with the updated metadata:

Here’s an example of how you can do this in Python:

from datetime import datetime
from zoneinfo import ZoneInfo
from pathlib import Path
from neuroconv.datainterfaces import OpenEphysRecordingInterface

Set the folder path to your Open Ephys data

folder_path = "path/to/your/openephys/data"

Initialize the interface

interface = OpenEphysRecordingInterface(folder_path=folder_path)

Extract metadata from the source files

metadata = interface.get_metadata()

Set the session start time if it cannot be inferred automatically

session_start_time = datetime(2020, 1, 1, 12, 30, 0, tzinfo=ZoneInfo("US/Pacific"))
metadata["NWBFile"].update(session_start_time=session_start_time)

Define your trials data

trials = [
{"start_time": 0.0, "stop_time": 1.0, "stimulus": "stimulus_1"},
{"start_time": 1.5, "stop_time": 2.5, "stimulus": "stimulus_2"},
# Add more trials as needed
]

Add trials information to the metadata

metadata["trials"] = {
"colnames": ["start_time", "stop_time", "stimulus"],
"table": trials
}

Set the path to save the NWB file

nwbfile_path = "path/to/save/your_file.nwb"

Run the conversion

interface.run_conversion(nwbfile_path=nwbfile_path, metadata=metadata)
In this example, the trials list contains dictionaries with start_time, stop_time, and stimulus keys. You can add more keys as needed to include additional trial metadata.

Make sure to replace "path/to/your/openephys/data" and "path/to/save/your_file.nwb" with the actual paths on your system.

If you have any more questions or need further assistance, feel free to ask! Have a great day!

This is not accurate. But honestly, I think it's a good idea, and it wouldn't be hard to implement.

Do you have any interest in helping implement the feature?

Yes.

Code of Conduct

@bendichter
Copy link
Contributor Author

We currently have an Excel and Csv reader for trials, but those are rarely used. Anyway, I like the idea of being able to pass the data to an interface in-memory, since we often are reading this data from esoteric .mat files and other types of formats where the data reading step is very user-specific.

I can see two options here

  1. Add this as a method to the BaseInterface.
    This would allow us to follow exactly the syntax the bot recommended, but might not be ideal from a perspective of separation of concerns. It would also require us to add super().add_to_nwbfile(nwbfile, metadata) to basically every DataInterface class.

  2. A separate interface. This interface would have no source files, and would only create a trials table based on the metadata. Create something like TimeIntervalsFromMetadataInterface which would know to read the metadata["trials'] field. We could create a thin wrapper with the name TrialsInterface or something like that for convenience. The advantages of this approach are that we could do it without altering any existing code and it's probably a better architecture in terms of separation of concerns. the downside of this approach is that it would require a bit more code and know-how on the user side. A user would need to create an NWBConverter or a ConverterPipe or create the NWB file with one interface and append to it with the other. Neither of these options are really terrible. Especially the last one- all it would really require is:

TrialsInterface().add_to_nwbfile(nwbfile, metadata)

Since this doesn't really even require an init, we could even make it a class method, so you could do:

TrialsInterface.add_to_nwbfile(nwbfile, metadata)

Then if you use an NWBConverter or a ConverterPipe and your metadata contains a trials table, we could automatically add TrialsInterface.

I think I'm leaning towards option 2.

@h-mayorquin
Copy link
Collaborator

I also vote for number 2.

@bendichter
Copy link
Contributor Author

bendichter commented Aug 15, 2024

For now I just decided to improve the docs to show how to add trials with PyNWB. I think this is basically the same amount of effort as adding metadata, so I don't think we gain much here by adding an interface.

Then theres the question of the NWB GUIDE. Having an interface could be helpful there, but we instead might just want to go a completely different direction and just have a dedicated page with dedicated logic for adding trials.

@h-mayorquin
Copy link
Collaborator

Should we close this issue? I rather close issues once they get to a somehow logical conclussion but I don't know how you prefer to handle this. I am asking to take this into account going into the future.

@bendichter
Copy link
Contributor Author

yeah, sounds good

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

Successfully merging a pull request may close this issue.

2 participants