Constrain shape to 3D to avoid inconsistency in usage

[CodyCBakerPhD]

related to similar conversations on ndx-microscopy

The ability for an object to have multiple shapes can often be confusing to users and their scripts written to analyze data, which expects it to follow certain consistencies

It also magnifies the complexity of any NWB Inspector checks that need to be written on such objects

Example: a user writes a script to meta-analyze every TwoPhotonSeries on DANDI - they prototyped it on one Dandiset that used a 3D shape - then it errors when they hit a file that uses a 4D convention

The current extension at line https://github.com/catalystneuro/ndx-binned-spikes/blob/main/spec/ndx-binned-spikes.extensions.yaml#L28-L29 allows for a 'single-unit' case for a 2D data shape. I would argue that this case is both somewhat rare and could be informative covered by the 3D case with a singleton dimension to cover that single unit case.

From discussion, you had reservations about a user writing their data that is single-unit and how they might be confused about why they must add the singleton - but you could always setup the PyNWB based class __init__ to accept 2D data as a duck type and fill this singleton behind the scenes. This effort to avoid confusion is largely for the consumers of data

Another alternative, which might be more effort than its worth, would be to make two separate objects for the 2D and 3D cases to be really explicit

[h-mayorquin]

From discussion, you had reservations about a user writing their data that is single-unit and how they might be confused about why they must add the singleton - but you could always setup the PyNWB based class init to accept 2D data as a duck type and fill this singleton behind the scenes. This effort to avoid confusion is largely for the consumers of data

This makes sense.

[h-mayorquin]

@CodyCBakerPhD

Right now main has the constrained version. I also wanted to implement your other comment which I think was a great idea!

• but you could always setup the PyNWB based class init to accept 2D data as a duck type and fill this singleton behind the scenes. This effort to avoid confusion is largely for the consumers of data

BUT, doing this turned out harder than I thought and I am not please with the result, see #7 where this is done.

The problem with it is that docval checks the shape of the argument BEFORE the __init__ so I can't have the strict in the reqiurements:

ndx-binned-spikes/src/pynwb/ndx_binned_spikes/__init__.py

Lines 64 to 69 in 68a1dc1

	{
	"name": "data",
	"type": "array_data",
	"shape": [(None, None, None), (None, None)],
	"doc": "The source of the data",
	},

In #7 we guarantee that the data will always be (number_of_units, number_of_trials, number_of_bins) which is good. The downside is that in docval (which is the spec, right?) the shape still needs to have the two options. Otherwise I can't take the data with shape (number_of_trials, number_of_bins) as input and then coerce it as you suggested it.

Maybe we will be able to do it once this happens:
hdmf-dev/hdmf#1072

Or do you know other way?

I am leaning to just accept (number_of_units, number_of_trials, number_of_bins) data until a better solution comes around instead of complicating the spec, what do you think?

[CodyCBakerPhD]

Otherwise I can't take the data with shape (number_of_trials, number_of_bins) as input and then coerce it as you suggested it.

Though I dislike them as general practice, you could decorate the __init__ before the docval decorator with a ducktyper decorator and it will intercept it before sending to docval

[CodyCBakerPhD]

That's if you feel that strongly about it. Otherwise strict input typing to match strict output typing. I'm still not convinced a single unit recording would be that common for this case

[h-mayorquin]

Though I dislike them as general practice, you could decorate the init before the docval decorator with a ducktyper decorator and it will intercept it before sending to docval

This is clever. Thanks for suggesting it, it did not occur to me before. That said, I think I believe you on this:

That's if you feel that strongly about it. Otherwise strict input typing to match strict output typing. I'm still not convinced a single unit recording would be that common for this case

And I would go with just not accepting that case.

[h-mayorquin]

OK, I will close this. As this is the way that is already in main and I don't intend to change it.