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

units/waveform_mean and units/waveform_sd missing sampling rate #273

Closed
bendichter opened this issue Jun 13, 2019 · 5 comments · Fixed by #323
Closed

units/waveform_mean and units/waveform_sd missing sampling rate #273

bendichter opened this issue Jun 13, 2019 · 5 comments · Fixed by #323
Assignees

Comments

@bendichter
Copy link
Contributor

Maybe I am missing something but I do not think it is possible to determine the sampling rate of waveform_mean and waveform_sd in the Units table. To solve this, we could add an optional waveform_sampling_rate attribute to the Units table that is required if these columns are present.

@tjd2002
Copy link
Contributor

tjd2002 commented Jun 17, 2019 via email

@rly rly added this to To do in Schema 2.1 Jun 18, 2019
@rly
Copy link
Contributor

rly commented Oct 11, 2019

we could add an optional waveform_sampling_rate attribute to the Units table that is required if these columns are present.
Same goes for the unit field.

I agree with adding these fields. The waveform_unit attribute should be fixed to "volts". I also think we should note in the doc of waveform_mean and waveform_sd that those data should be stored in actual converted volts, i.e. not require a conversion factor.

@bendichter
Copy link
Contributor Author

This has come up twice more in contexts that do not involve the waveforms but where extracellular electrophysiology processing tools expect to have access to the sampling rate of the original data. Because of this, I would prefer we name this field something a little more general, like sampling_rate_of_acquisition. I can't think of a situation where you would need this information and wouldn't also need spikes, so I think it's still fine as a field of the Units table, but might be better off as an optional dataset in /general/extracellular_ephys just in case.

@rly
Copy link
Contributor

rly commented Nov 4, 2019

I prefer having the sampling rate attached to the group/dataset of the associated data, rather than as a general dataset in general/extracellular_ephys, to reduce ambiguity over what it corresponds to and to prevent data duplication if an ElectricalSeries is provided.

@rly
Copy link
Contributor

rly commented Nov 6, 2019

After discussion with @oruebel, instead of putting the waveform_sampling_rate and waveform_unit attributes on the Units table, we decided it would be more appropriate to put those attributes on the individual columns that they reference (waveform_mean and waveform_sd). The sampling rate would be duplicated (in almost all cases), but it is more appropriate to have them on the datasets that they refer to in case neither are provided or one is provided without the other.

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 a pull request may close this issue.

3 participants