NeurodataWithoutBorders / matnwb

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

validation requires electrode ElementIdentifiers to be int, but no such type in Matlab #341

Closed emilyasterjones closed 2 years ago

emilyasterjones commented 2 years ago

This bug appears when validating general_extracellular_ephys_electrodes. Following the MatNWB tutorial, which builds an electrodes table but doesn't manually specify the ids, you get:

ElementIdentifiers (general/extracellular_ephys/electrodes/id): incorrect type - expected 'int', got 'int8',
 DynamicTable/id (general/extracellular_ephys/electrodes/id): incorrect type - expected 'int', got 'int8']

Matlab only has int types where the number of bytes is specified (int8, int16, etc) so it's not possible for an int to be stored here.

bendichter commented 2 years ago

@emilyasterjones int is usually just shorthand for int32. The schema defines the minimum precision of a data type, so we need the data to be >= 32 bits. I think the problem here is that MatNWB is demoting integer types to int8. See #339

lawrence-mbf commented 2 years ago

Does this still occur with the merging of #342?

emilyasterjones commented 2 years ago

Sorry, I don't have time to test this right now. If you use the code in the tutorial it should replicate the issue.

lawrence-mbf commented 2 years ago

This appears to be fixed as well though I could be validating this incorrectly using PyNWB 2.0.0. Can @bendichter or @cechava confirm that this works?

cechava commented 2 years ago

@ln-vidrio this issue is fixed. The file now passes validation.

cechava commented 2 years ago

fixed with #342