Question: Should dtype for TimeSeriesReferenceVectorData's `idx_start` and `count` fields be uint instead of int?

[ehennestad]

The dtype of idx_start and count in TimeSeriesReferenceVectorData is currently int32:

nwb-schema/core/nwb.base.yaml

Lines 9 to 16 in 1cbd9a8

	dtype:
	- name: idx_start
	dtype: int32
	doc: Start index into the TimeSeries 'data' and 'timestamp' datasets of the referenced
	TimeSeries. The first dimension of those arrays is always time.
	- name: count
	dtype: int32
	doc: Number of data samples available in this time series, during this epoch

This allows for negative numbers which does not make sense for either an index into a vector or a count. Should these be changed to uint32?

[ehennestad]

Ok, I found this which I assume refers to the use of negative integers for idx_start and count in TimeSeriesReferenceVectorData:

nwb-schema/core/nwb.icephys.yaml

Lines 309 to 320 in 1cbd9a8

	name: intracellular_recordings
	doc: A table to group together a stimulus and response from a single electrode and
	a single simultaneous recording. Each row in the table represents a single recording
	consisting typically of a stimulus and a corresponding response. In some cases,
	however, only a stimulus or a response is recorded as part of an experiment.
	In this case, both the stimulus and response will point to the same TimeSeries
	while the idx_start and count of the invalid column will be set to -1, thus, indicating
	that no values have been recorded for the stimulus or response, respectively.
	Note, a recording MUST contain at least a stimulus or a response. Typically the
	stimulus and response are PatchClampSeries. However, the use of AD/DA channels
	that are not associated to an electrode is also common in intracellular electrophysiology,
	in which case other TimeSeries may be used.

Should this "null case" be described in the documentation for the TimeSeriesReferenceVectorData?

[oruebel]

Should this "null case" be described in the documentation for the TimeSeriesReferenceVectorData?

I agree. I think it would be useful to mention this behavior there as well.

[oruebel]

This allows for negative numbers which does not make sense for either an index into a vector or a count. Should these be changed to uint32?

As you already mentioned, -1 is currently being used to indicated invalid/none references. It would be really helpful if integers would have a standard NaN value like floats do, but they unfortunately do not, so we had to define what an invalid index looks like. PyNWB has a separate class to represent individual references which has isvalid and empty to check for and create invalid references

[ehennestad]

Are you implying that a similar class in MatNWB would facilitate changing the schema?

[oruebel]

Are you implying that a similar class in MatNWB would facilitate changing the schema?

The TimeSeriesReference class in PyNWB is mainly a convenience class to make it easier for users to interact with individual references, e.g., to access the selected range in a TimeSeries, to make them easier to use in Pandas tables etc.. If the schema would change, the class might help PyNWB with ensuring backward compatibility, but that is not the reason we created that class. The main intent was to enhance usability for users.

I am not sure whether changing the schema to use uint instead of int index values would help because: 1) we would need still need define a way to indicate none/missing references and 2) it would require additional logic (at least in PyNWB) to support both the current and whatever the new behavior would be.