catalystneuro / ndx-binned-spikes

NWB extension to store binned spikes (e.g. a non averaged PSTH) with respect to an time events
MIT License
0 stars 0 forks source link

Add optional link to events source #3

Open CodyCBakerPhD opened 6 months ago

CodyCBakerPhD commented 6 months ago

You're adding a link to the units table (or in #1 a dynamic table region to the units table) but it would be nice to have a link of some sort to the events we're aligning that neural data to

As discussed this could be a time column on a dynamic table (which could use either (i) an extra field to specify the name of the column on that dynamic table region used to align or (ii) extend the HDMF dynamic table region to specify not just row indices but also column names), or certain objects from ndx-events, or even certain timestamps from a TimeSeries such as an IndexSeries (in which case use a TimeSeriesReference as the link)

CodyCBakerPhD commented 6 months ago

This could technically also replace the current event-timestamps (https://github.com/catalystneuro/ndx-binned-spikes/blob/main/spec/ndx-binned-spikes.extensions.yaml#L40) by linking to the timestamps used for the bin alignment so the timestamps data is not duplicated

I'm more on the fence about that one though - it can be nice to have the timestamps attached explicitly here for easy access instead of having to follow a link

h-mayorquin commented 6 months ago

My initial plan was to have the two options and use post_init checks for this. But yes, I like to preserve the ability to build an NWB object simply. That meaning, without having to build x or y first to instantiate it.

h-mayorquin commented 6 months ago

Ok, so this can't be done with the current automatic mechanism to define a class BinnedAlignedSpikes = get_class("BinnedAlignedSpikes", "ndx-binned-spikes"). That is, I need some sort of post_init check to verify that only one of the following is passed: 1) event-timestamps or 2) a reference to any other object in the spec that represent event timestamps (e.g. start_time in a dynamic column, ndx-events, etc).

I am gona add documentation of the current use case as a the next prority and come back to this once I explore the DiCarlo data as this is a nice to have.

CodyCBakerPhD commented 6 months ago

I just meant in the schema, there could be an optional link to the actual object being used as a reference for the alignment, such as an IndexSeries, column of DynamicTable, etc. in addition to the explicit event timestamps. To clarify that there is a relationship between the two objects

For example, say you have a pretty complicated trials table with lots of conditions per trial, and you want to make several of these alignment objects you're designing for various columns. Let's also say you have other stuff like optogenetics or visual stimuli in your stimulus areas of the NWB file - if your extension objects can link to the trials table (and in a best-case scenario, the particular column of that table) that helps remove any confusion about where the source of the event timestamps might be and also is a hint to a data reuser to 'go check out that linked object for more details'

h-mayorquin commented 6 months ago

I just meant in the schema, there could be an optional link to the actual object being used as a reference for the alignment, such as an IndexSeries, column of DynamicTable, etc. in addition to the explicit event timestamps.

OK, thanks. Making it in addition and not xor should be easier.

CodyCBakerPhD commented 1 week ago

@h-mayorquin Feel free to close w/e you decide