Cannot extend OptogeneticSeries to have 2D data

[weiglszonja]

@alessandratrapani is working on an extension for holographic stimulation data. We would like to extend OptogeneticSeries to have 2D data (n_time x n_roi), however we got "incorrect shape" (see below) after changing "data" to allow for 2D.

Original issue opened here

Extension specification:

https://github.com/catalystneuro/ndx-holographic-stimulation/blob/82fba2b7763a06ad35563252e05171fed269c372/src/spec/create_extension_spec.py#L63-L77

When running test_example_usage(), it displays this error message:

ValueError: CustomClassGenerator.set_init.<locals>.__init__: incorrect shape for 'data' (got '(100, 12)', expected '(None,)')

It still expects the data shape of the OptogeneticSeries (None,), even if (None, None) was defined as the new data shape. Extending HolographicSeries from TimeSeries resolves the error.

[oruebel]

I believe this is because the OptogeneticSeries restricts the shape of data to be 1D here:

nwb-schema/core/nwb.ogen.yaml

Lines 6 to 11 in ec0a879

	- name: data
	dtype: numeric
	dims:
	- num_times
	shape:
	- null

Similar to classes in programming, child-classes can restrict properties or can add new properties but child classes cannot relax properties. E.g., if you parent class is Car and it says a car can have [3,4,, ..n] wheels, then a child class RaceCar can restrict the number of wheels and say that a RaceCar must have 4 wheels. However, a child-class of RaceCar can then not say that it has 5 wheels, because then it would no longer be a RaceCar.

If you want to build of OptogenticSeries and need 2D data, then I think we'd need to update OptogeneticSeries to allow for this behavior.

that says a car must have 4 wheels then a child class, e.g., RaceCar, cannot then change this to

[weiglszonja]

Thank you for the clarification, I understand now. @alessandratrapani what do you think about this?

[alessandratrapani]

Thank you so much for the explanation @oruebel. It's true that the holographic stimulus could be seen as a type of optogenetic stimulus. Still, they are also very different concepts in the way they deliver light to the sample: by constraining optogenetic series being one dimension I guess they were referring only to widefield optogenetic stimulation (single photon excitation), but you could have patterned photostimulation (that usually uses two-photon excitation) that needs a second dimension to be described since they target single ROI.

[oruebel]

Updating OptogenticSeries to allow 2D data should not break backward compatibility. As such, if relaxing this constraint is required for this extension and make OptogeneticSeries more broadly useful, then I think that is a potential option. I'm not familiar with holographic stimuli, so I'm not sure whether it makes more sense to create a new type for this or extend OptogenticSeries. @bendichter @rly thoughts?

[bendichter]

I'd prefer to extend OptogeneticSeries, since this would mark the file as having optogenetic stimulus data in a way that is generalized across different types of stimulation.

[oruebel]

I'd prefer to extend OptogeneticSeries, since this would mark the file as having optogenetic stimulus data in a way that is generalized across different types of stimulation.

I think relaxing OptogeneticSeries to allow for 2D should be fine

[rly]

I agree. I think relaxing OptogeneticSeries to allow for 2D should be fine. I'll start a PR.

Is this still needed in light of the new path forward with ndx-patterned-ogen? @alessandratrapani

[alessandratrapani]

Is this still needed in light of the new path forward with ndx-patterned-ogen? @alessandratrapani

If we move forward with the TimeInterval approach, we will not extend from OptogeneticSeries anymore. But maybe it is worth to leave the option open.