Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] add enhanced subject #80

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[WIP] add enhanced subject #80

wants to merge 3 commits into from

Conversation

bendichter
Copy link
Contributor

No description provided.

@bendichter
Copy link
Contributor Author

@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
Copy link
Contributor Author

@lawrence-mbf
Copy link
Collaborator

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

@bendichter
Copy link
Contributor Author

@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).

@lawrence-mbf
Copy link
Collaborator

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

@bendichter
Copy link
Contributor Author

Yeah that's reasonable

@yarikoptic
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants