-
Notifications
You must be signed in to change notification settings - Fork 16
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
Define ElectrodesTable and FrequencyBandsTable types #337
base: dev
Are you sure you want to change the base?
Conversation
This won't cause validation or IO errors for files that already exist? If this can be done without disrupting users I think it's a good idea |
I think on the PyNWB the validation and read of existing files can be handled. @bendichter does this change anything for MatNWB? |
@oruebel I don't know, possibly. I can check |
I'm having trouble getting the matnwb to work. They are several schema changes behind at this point. |
This break |
I would prefer to have the convention that tables are called Tables, i.e. this would be an |
@rly status update: this is working on the matnwb side |
I would prefer this convention as well. However, we already have the |
I was talking about a convention for the neurodata_type name, not the object name, so it's not an issue for "epochs", "trials", "invalid_times". I agree we have types that break this convention, but I think we should use it going forward. At least when I create extensions I am going to try to use this naming convention for children of DynamicTable. At any rate, this schema change looks good to me besides that. |
I renamed the types and suggested a renaming of the icephys extension types: oruebel/ndx-icephys-meta#27 If we go forward with this convention, I also suggest renaming |
@rly that sounds scary. I'm worried about backwards compatibility, esp for matnwb |
Fix #336