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] changes to schema #92

Merged
merged 9 commits into from
Jan 18, 2019
Merged

[WIP] changes to schema #92

merged 9 commits into from
Jan 18, 2019

Conversation

bendichter
Copy link
Contributor

currently generateCore throws error:

Dot indexing is not supported for variables of this type.

Error in file.fillValidators>fillUnitValidation (line 91)
            sg_namespace = namespacereg.getNamespace(sg.type).name;

Error in file.fillValidators (line 12)
        validationBody = fillUnitValidation(nm, prop, namespacereg);

Error in file.fillClass (line 87)
validatorFcns = file.fillValidators(allprops, classprops, namespace);

Error in file.writeNamespace (line 15)
        fwrite(fid, file.fillClass(k, namespace, processed, ...

Error in generateExtension (line 42)
file.writeNamespace(extmap(extSchema.name));

Error in generateCore (line 22)
generateExtension(core);
 
Reference to non-existent field 'internal'.

@bendichter
Copy link
Contributor Author

This error appears to be in constructing Devices.

@lawrence-mbf
Copy link
Collaborator

The inclusion of the LabMetaData type has broken elision for general. I'll see if I can gracefully split that into multiple properties.

* change OgenStimSite.excitation_lambda str to float
* change OgenStimSite.device from str to link
@bendichter
Copy link
Contributor Author

bendichter commented Jan 15, 2019

I see. It's sort of a funny type because its sole purpose is to be extended, but it saves people from having to extend NWBFile itself, which I imagine would cause bigger problems. Thanks for handling this. I'm also pushing some changes that I know will be necessary (though I can't test them while generateCore is broken)

@bendichter
Copy link
Contributor Author

@ln-vidrio any progress here?

@lawrence-mbf
Copy link
Collaborator

I'm fixing tests now but it's essentially a schema reading adjustment so I'd imagine it will also break most if not all tests and utilities.

@bendichter
Copy link
Contributor Author

yikes. OK... thanks for getting this in

@lawrence-mbf
Copy link
Collaborator

So I'll summarize script changes:
Due to changes in elision rules, the properties for any class with Sets have been restructured.
Basically, if a property is "missing" it may've been "hoisted" as its parent property. So you can't find "general_labmetadata" as a property because it's been hoisted into the "general" property. Hopefully, this will only cause syntax errors in the scripts.

@lawrence-mbf lawrence-mbf merged commit eb37be0 into master Jan 18, 2019
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.

3 participants