-
Notifications
You must be signed in to change notification settings - Fork 25
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
Initialize Literature Annotation domain model #192
Conversation
@apdavison
|
v0.2.0 adds the The goal was to have feedback first but it was needed to add this to make the data findable in Nexus. |
"@id": "AnnotationShape", | ||
"@type": "sh:NodeShape", | ||
"nodeKind": "sh:BlankNodeOrIRI", | ||
"class": "oa:Annotation", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we replace this with a comment? Or add a comment to point out that we capture the oa:Annotation description in these shapes? Also, can "sh:class" be used outside property shapes? @MFSY
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did it to so that oa:Annotation
is in @type
for all instances using directly or indirectly AnnotationShape
.
{ | ||
"path": "oa:hasTarget", | ||
"name": "Target", | ||
"description": "The relationship between an Annotation and its Target.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe provide description in sentence case (i.e. annotation and target lower case) @MFSY
{ | ||
"path": "oa:hasBody", | ||
"name": "Body", | ||
"description": "The relationship between an Annotation and its Body.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
annotation and body lower case?
"@id": "TargetShape", | ||
"@type": "sh:NodeShape", | ||
"nodeKind": "sh:BlankNodeOrIRI" | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this shape necessary since there are no property shapes defined in it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did it to make explicit the specialization by SelectorTargetShape
as the latter is not the only kind of oa:hasTarget
. Besides I did it because I wanted to have a shape for this property in the generic nsg:AnnotationShape
.
"@id": "BodyShape", | ||
"@type": "sh:NodeShape", | ||
"nodeKind": "sh:BlankNodeOrIRI" | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this shape necessary since there are no property shapes defined in it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as nsg:TargetShape
.
"minCount": 1 | ||
} | ||
] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of creating the ContributionShape here, the existing agent shape https://github.com/INCF/neuroshapes/blob/master/modules/core/src/main/resources/schemas/neurosciencegraph/core/agent/v0.1.0.json could be used as value of the node "nsg:contribution" in the property shapes of this schema
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did it to enforce the fact that we want to have IRI of agents so that they come from a defined list, i.e. are not defined in this payload. Besides, the existing shapes for agents are not in a good 'shape':
https://github.com/BlueBrain/nexus-prov/blob/master/modules/prov/src/main/resources/schemas/nexus/provsh/agent/v1.0.0.json
https://github.com/BlueBrain/nexus-prov/blob/master/modules/prov/src/main/resources/schemas/nexus/prov/agent/v1.0.0.json
"@context": [ | ||
"{{base}}/contexts/neurosciencegraph/core/schema/v0.2.0", | ||
{ | ||
"@base": "{{base}}/schemas/neurosciencegraph/commons/selectors/v0.2.0/shapes/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to rename it to commons/selector/v0.2.0 to have it consistent with other schemas (i.e. schema name singular)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I remember correctly, @MFSY said to use the plural form for the ones where I did this. To be confirmed.
"description": "The class of the Selector.", | ||
"minCount": 1, | ||
"maxCount": 1 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of this property shape "Selector type"? Aren't you enforcing the type anyways through the "class" key in the individual selector shapes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, through the sh:class
constraint in the specialised SelectorShapes
, an rdf:type
is already required. However, I also wanted to have a generic SelectorShape
reusable by others and a oa:Selector
requires a rdf:type
.
"name": "End offset", | ||
"description": "The end position of the segment of text. The character is not included within the segment.", | ||
"datatype": "xsd:integer", | ||
"minInclusive": 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe put minInclusive: 1 since the value can never be 0 (because the value is not included in the segment and the oa:start property needs to be less than the oa:end property)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. I have just done it because this is how it is described in https://www.w3.org/TR/annotation-vocab/#end.
"maxCount": 1 | ||
}, | ||
{ | ||
"path": "dcterms:conformsTo", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a description?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I need to add it.
"name": "Quantity type", | ||
"description": "The type of the measured quantity represented by the variable.", | ||
"minCount": 1, | ||
"maxCount": 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Longer term, this shape should probably re-use the TypedLabeledOntologyTerm schema (https://github.com/INCF/neuroshapes/blob/master/modules/commons/src/main/resources/schemas/neurosciencegraph/commons/typedlabeledontologyterm/v0.1.4.json) with an included OntologyTermShape for the quantity types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. To be seen with @MFSY for the specific XyzOntologyTermShape.
"@context": [ | ||
"{{base}}/contexts/neurosciencegraph/core/schema/v0.2.0", | ||
{ | ||
"@base": "{{base}}/schemas/neurosciencegraph/core/selectors/v0.2.0/shapes/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to rename it to core/selector/v0.2.0 to have it consistent with other schemas (i.e. schema name singular)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment for selectors above.
"@context": [ | ||
"{{base}}/contexts/neurosciencegraph/core/schema/v0.2.0", | ||
{ | ||
"@base": "{{base}}/schemas/neurosciencegraph/commons/variables/v0.2.0/shapes/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to rename it to commons/variable/v0.2.0 to have it consistent with other schemas (i.e. schema name singular)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment made for selectors
.
"@context": [ | ||
"{{base}}/contexts/neurosciencegraph/core/schema/v0.2.0", | ||
{ | ||
"@base": "{{base}}/schemas/neurosciencegraph/core/variables/v0.2.0/shapes/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to rename it to core/variable/v0.2.0 to have it consistent with other schemas (i.e. schema name singular)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment for selectors above.
"class": "nsg:AreaTarget" | ||
} | ||
], | ||
"maxCount": 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I understood, the Web Annotation Data Model (https://www.w3.org/TR/annotation-model/#bodies-and-targets) specifies, that the minCount for a target is 1 while there is no minCount on the body (see excerpts below). Should the property shape oa:hasTarget
hence have a minCount of 1 and the minCount constraint on the oa:hasBody
property shape below be removed?
The relationship between an Annotation and its Target.
There must be 1 or more target relationships associated with an Annotation.
The relationship between an Annotation and its Body.
There should be 1 or more body relationships associated with an Annotation but there may be 0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oa:hasTarget
has already a sh:minCount
of 1 because ParameterAnnotationShape
is specializing AnnotationShape
where there is this constraint.
oa:hasBody
has sh:minCount
of 1 because it is a ParameterAnnotationShape
aso it needs parameters and I wanted to differentiate it from the annotations of facts which are not modeled yet and which have not parameters.
"@type": "sh:NodeShape", | ||
"nodeKind": "sh:BlankNodeOrIRI", | ||
"targetClass": "nsg:PointValueParameter", | ||
"comment": "Parameter shape for the value(s) of an unidimensional variable (i.e., not establishing a relationship between a dependant and an independent variable).", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a unidimensional
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will correct it.
More general comments:
|
] | ||
} | ||
] | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, AlgebraicValueShape could be modelled like the AgeShape in the subject schema:
{
"@id": "this:AgeShape",
"@type": "sh:NodeShape",
"property": [
{
"path": "nsg:period",
"name": "Period",
"in": [
"Pre-natal",
"Post-natal"
],
"minCount": 1,
"maxCount": 1
},
{
"path": "schema:value",
"name": "Age value",
"node": "{{base}}/schemas/neurosciencegraph/commons/quantitativevalue/v0.1.0/shapes/QuantitativeValueShape",
"minCount": 1,
"maxCount": 1
}
]
}
by extending the QuantitativeValueShape with an nsg:statistic property shape (just like the AgeShape extends the QuantitativeValueShape with the nsg:period property shape).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QuantitativeValueShape
is broader. We might want to use it.
https://github.com/INCF/neuroshapes/blob/master/modules/commons/src/main/resources/schemas/neurosciencegraph/commons/quantitativevalue/v0.1.2.json
- Restricted the domain to annotating entities based on web annotation data model - Used nsg namespace instead of oa for hasTarget, hasBody, hasSource,hasSelector, Annotation - Removed targetClass assertion from all schemas in commons domain - Added a seeAlso links towards the web annotation recommendation - Used schema:value instead of nsg:text and rdf:value - Renamed IndexSelectorShape to DocumentIndexSelectorShape - Directly use DocumentIndexSelectorShape instead of FigureSelectorShape - Removed FigureSelectorShape because it does not add anything new on top of DocumentIndexSelectorShape - Removed EquationSelectorShape - Renamed AreaSelectorShape to DocumentAreaSelectorShape - Renamed selectors schema with annotationselector - Removed @base and use this instead - Removed the different *TargetShape in the core domain - Removed core/EquationTargetShape - Removed selector schema from core domain
Hello,
Here is the first iteration of the Literature Annotation domain model.
It's goal is to achieve #51. It is related to #43 and #49.
Created SHACL shapes are compliant with W3C Web Annotation recommendations.
Known current limitations:
1 - Annotation of facts are not modelled yet. These annotations do not have parameters.
2 - Literature references are not modelled yet. Reason: prioritization and some work has already been done on the subject in #43. TODO: Contribute to @apdavison proposition.
3 - Provenance is not modelled explicitly yet. Implicitly, it is:
hasBody
),hasTarget
) which describe precisely where the information come from in the literature reference (hasSelector
) and which literature reference it is (hasSource
).contribution.agent
).4 - The NeuroCurator field 'relationship' which describes more precisely on what the measure of a parameter is has not been modelled yet.
5 -
rdf:value
anddcterms:conformsTo
has not been added to/contexts/neurosciencegraph/core/data/vx.x.x
yet.6 -
schema:unitCode
has been explicitly asked to be used instead ofschema:unitText
even though values corresponds to the later definition for now.7 - Literature references without DOI or PMID are currently not modelled for the
hasSource
property.Known tricky cases:
8 - Should
TargetShape
exists?9 - Should
BodyShape
exists?10 - There is no check at the SHACL level to declare invalid instances with a required property with an empty string as value.