NeurodataWithoutBorders / matnwb

A Matlab interface for reading and writing NWB files
BSD 2-Clause "Simplified" License
50 stars 32 forks source link

[Bug]: #430

Closed vikramvijayan closed 2 years ago

vikramvijayan commented 2 years ago

What happened?

I need to make a timeseries with only 1 entry (it is a sparse behavioral annotation of my data). I make both the data and timestamp a single entry and MatNWB makes the file fine, but I get an error when running nwbinspector.

Please let me know what is the best option for me to include this behavioral annotation. I cannot upload my data to DANDI because of this error (the NWB file itself looks fine other than this error).

Steps to Reproduce

spatial_series_ts12 = types.core.TimeSeries( ...
            'data', 12.3, ...
            'description', 'XYZ', ...
            'data_unit', 'time', ...
            'timestamps', 12.3);

Error Message

TypeError: TimeSeries.init: incorrect type for 'data' (got 'float64', expected 'ndarray, list, tuple, Dataset, StrDataset, HDMFDataset, AbstractDataChunkIterator, DataIO or TimeSeries'), incorrect type for 'timestamps' (got 'float64', expected 'ndarray, list, tuple, Dataset, StrDataset, HDMFDataset, AbstractDataChunkIterator, DataIO or TimeSeries')"

Operating System

Windows

Matlab Version

2021A

Code of Conduct

oruebel commented 2 years ago

Based on the error, it seems that the data is stored as a single scalar value, rather than as an array. Instead of providing the data as a single value, I believe you need to input it as an array instead. I think the following should work:

spatial_series_ts12 = types.core.TimeSeries( ...
            'data', [12.3, ], ...
            'description', 'XYZ', ...
            'data_unit', 'time', ...
            'timestamps', [12.3, ]);

Quick question about the data. In the description you mention that this is an 'XYZ' location but the data is a scalar. If this is indeed a spatial series then https://neurodatawithoutborders.github.io/matnwb/doc/+types/+core/SpatialSeries.html could potentially be a more appropriate type. Also, I noticed that the timestamps have the same value as the data, is this expected?

CodyCBakerPhD commented 2 years ago

@oruebel I think problem at the heart of it is MATLAB doesn't seem to distinguish the data type at all between the commands

a=1
b=[1]
c=[1,]

a==b
> 1 (logical)

b==c
> 1 (logical)

It only ends up as an array if there is, in fact, more than one item present in the array.

vikramvijayan commented 2 years ago

@oruebel see @CodyCBakerPhD 's comment.

Also, "XYZ" was just meant to be a placeholder for an actual description (not coordinate information). Sorry for that confusion.

I made the 'data' and 'timestamp' the same because it is simple a behavioral annotation (when an animal laid an egg). Alternatively I could put data = 1 and timestamp = 12.3s. I'm open to both options but that won't fix the initial problem at hand.

oruebel commented 2 years ago

I think problem at the heart of it is MATLAB doesn't seem to distinguish the data type at all between the commands

Thanks for the clarification. I did not know that. That seems to be a question for @lawrence-mbf

lawrence-mbf commented 2 years ago

This is fixable, it just has to do with how datasets know they must always be non-scalar as derived from the schema.

Is the case below the only time in the schema where a scalar dataset would be allowed?

shape:
- - 1
- - null 
  - null
... etc
vikramvijayan commented 2 years ago

Thanks! Does this mean that once the commit is merged I can update matNWB and be ready to go?

lawrence-mbf commented 2 years ago

Unfortunately, you'd have to recreate the file but otherwise yes.

vikramvijayan commented 2 years ago

No problem at all! I'll monitor to see when the code has been updated!

bendichter commented 2 years ago

@lawrence-mbf it sounds like the solution you are planning is ideal- defer to the schema to see if the dataset should be a vector. If that it difficult, another option could be to use DataPipe, but it seems like that would be far less user-friendly than what you are thinking.

lawrence-mbf commented 2 years ago

@vikramvijayan Sorry for letting this fall through. Would it be possible for you to check if this resolves the warning? The code is currently still in the PR.

vikramvijayan commented 2 years ago

Thanks @lawrence-mbf ! How would I go about downloading the code that is in PR? Downloading a .zip would be the easiest and I can take it from there.

lawrence-mbf commented 2 years ago

@vikramvijayan You should be able to download from this URL: https://github.com/NeurodataWithoutBorders/matnwb/tree/430-force-dataset-array-shape.

vikramvijayan commented 2 years ago

Thanks @lawrence-mbf , it appears the updated code works!