matnwb icon indicating copy to clipboard operation
matnwb copied to clipboard

[WIP] add enhanced subject

Open bendichter opened this issue 7 years ago • 7 comments

bendichter avatar Nov 10 '18 20:11 bendichter

@ln-vidrio

I'm playing with including more information in the Subject object, and I am running into an issue that I think may be a bug in matnwb. On this branch, when I run types.core.Subject I get:

Error using types.util.checkDtype (line 107)
Property `surgeries` must be a types.core.Surgeries.

Error in types.core.Subject/validate_surgeries (line 134)
        val = types.util.checkDtype('surgeries', 'types.core.Surgeries',
        val);

Error in types.core.Subject/set.surgeries (line 99)
        obj.surgeries = obj.validate_surgeries(val);

Error in types.core.Subject (line 64)
        obj.surgeries = p.Results.surgeries;

Surgeries is optional (quantity=: '?'), so I don't think I should be getting this error.

bendichter avatar Nov 10 '18 21:11 bendichter

fyi @oruebel

https://github.com/NeurodataWithoutBorders/nwb-schema/issues/148

bendichter avatar Nov 10 '18 22:11 bendichter

119c4e11935ab5fff5fd2ca75a58e1f1fcde22dd This seems to fix it. Requires generating again.

lawrence-mbf avatar Nov 12 '18 15:11 lawrence-mbf

@ln-vidrio thanks! That works but now I think I found another quantity bug.

The following code:

subject = types.core.Subject('surgeries',types.core.Surgeries());

file = nwbfile('session_description', 'a test NWB File', ...
'identifier', 'blockname', ...
'session_start_time', datestr(now, 'yyyy-mm-dd HH:MM:SS'), ...
'file_create_date', datestr(now, 'yyyy-mm-dd HH:MM:SS'),...
'general_subject',subject);

nwbExport(file, 'test.nwb')

runs but should throw an error.

types.core.Surgery is defined as a group of types.core.Surgeries with quantity: +, which means Surgeries must contain "one or more" Surgery objects. This code creates Subject.surgeries.surgery with an empty set, which is a violation of the schema. I would prefer if this was caught in the constructor, but it could also be caught during nwbExport (it currently is not).

bendichter avatar Nov 12 '18 22:11 bendichter

8a914802e655a850d3f40aa7c9b62c4d0bebddbd should be correct. I opted against checking on construction since these are also called when reading nwbfiles.

lawrence-mbf avatar Nov 12 '18 22:11 lawrence-mbf

Yeah that's reasonable

bendichter avatar Nov 12 '18 22:11 bendichter

is it really still WiP @bendichter or might want to be closed (or converted to draft at least)?

yarikoptic avatar Feb 24 '23 19:02 yarikoptic