-
Notifications
You must be signed in to change notification settings - Fork 85
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
Associate cluster waveforms and electrode table regions with unit times #431
Comments
What about a type that maps between the different IDs? It could be a table with two columns. For example, in the ElectrodeTable to UnitTimes case, the first column would be an id from the electrode table and the second column would be an id from UnitTimes.unit_id. |
I support a separate class to represent Units (independent of UnitTimes/ClusterWaveforms).And NB we definitely want to refer to electrodetableregions and not just electrodeids. What is the reason to separate this list of Units from the UnitMetrics class as proposed at PR #435? Are there cases where there will not be a 1:1 correspondence between UnitMetrics and Units? |
The way that I am currently leaning towards implementing Units is as a group with two datasets:
This would allow us to attach electrode groups, event times, waveforms and other data to units while preserving the type of the attached data. We could then units obtained from different modalities (ecephys, ophys, models) in a cohesive framework. I think that @tjd2002's suggestion is reasonable. The UnitMetrics could be attached to the file as a subgroup of the Units |
These all sound like good solutions. I just want to make sure that whatever we pick can support, either by links or some other means, indicating the relationship between individual units and:
|
@bendichter I think that UnitwiseMetrics covers scalar values such as cell type and mean firing rate. The solution I propose above is flexible enough to support the others as long as we create types for the ones that are not currently in spec, such as receptive field. If there are no objections, I will go ahead and implement this. |
I like the fact that the proposed solution extends to other unit-wise data.
One question (perhaps for the core developers) is whether this solution is idiomatic (‘NWB-ish’). It seems to approach some of the proposals for composable extensions made at the Allen Hackathon by @campagnola et al. (I.e. no need to subclass Unit in order to associate new types of Unit-wise data via an extension). Perhaps this could be a good test case for this approach.
…Sent from my phone
On Apr 13, 2018, at 4:35 PM, NileGraddis ***@***.***> wrote:
@bendichter I think that UnitwiseMetrics covers scalar values such as cell type and mean firing rate. The solution I propose above is flexible enough to support the others as long as we create types for the ones that are not currently in spec, such as receptive field.
If there are no objections, I will go ahead and implement this.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
For discussion of extensions see: NeurodataWithoutBorders/nwb-schema#141 |
I like the solution proposed by @NileGraddis; it's simple, it solves the composition problem (and solves it in a way that makes sense when you are dealing with hundreds/thousands of units), and unlike #141 it does not require any changes to NWB. Things to consider:
|
The |
UnitTimes provide a nice way to store event times associated with sorted units. There should be a way to associate mean/std waveforms (such as those in ClusterWaveforms) as well as electrodes (such as those in ElectrodeTableRegion) to the units defined by a UnitTimes container.
Since UnitTimes supports simulated data and ophys data as well as ephys data, the solution to this problem should probably involve referencing units in UnitTimes from other containers rather than altering UnitTimes itself.
The text was updated successfully, but these errors were encountered: