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

Add additional validation checks #26

Merged
merged 8 commits into from
Dec 5, 2018
Merged

Add additional validation checks #26

merged 8 commits into from
Dec 5, 2018

Conversation

cboettig
Copy link
Member

@mbjones Can you take a quick look through this when you get a chance.

I believe this should address #25 by implementing the additional validation checks, as I understood them (modulo any handling of system. Currently I just treat all ids throughout the document directly, while I think technically I should be permitting identical ids that have different systems, and likewise making sure that references describes match the system and not just the id value, right?)

I believe I have understood the rule correctly about the use of id or references regarding an annotation (the annotation must have a subject, and only one can be the subject), but I'm not 100% sure. In particular, it looks like the current (but 5 mo stale) eml-data-paper.xml fails this test because there is a child annotation on a dataset node that has no id, and no references on the annotation. You'll see in this PR I've taken the liberty of modifying my local copy of that test file.

Also, I wasn't clear if the packageId needed to be included in the list of ids that had to be unique (or similarly, if references and describes were allowed to reference the packageId instead of an id). Currently I have followed the instructions literally, so packageId has to exist but that is all, it's not part of the other checks.

experimental or stale files removed or moved to notebook
@cboettig
Copy link
Member Author

cboettig commented Dec 5, 2018

From discussions elsewhere, sounds like this checks out well enough so far, so I'm going to merge this into master. Validation could still be improved, in particular, in testing (with more invalid files) and error message quality, but will coordinate this with @mbjones once the updates to the official java validator are complete as well.

@cboettig cboettig merged commit d786856 into master Dec 5, 2018
@cboettig cboettig deleted the additional-validation branch January 23, 2019 04:43
@cboettig cboettig mentioned this pull request Jan 26, 2019
3 tasks
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.

None yet

1 participant