Implement Check Phase Ruleset #46
Conversation
… somehow clarified.
|
here as well I think we tackle the outstanding rules in another PR |
douglowe
left a comment
douglowe
left a comment
There was a problem hiding this comment.
It's looking good - just a couple of changes to make I think.
| sh:description "CheckValue MUST have a human readable name string of at least 20 characters." ; | ||
| sh:path schema:name ; | ||
| sh:datatype xsd:string ; | ||
| sh:minLength 20 ; |
There was a problem hiding this comment.
I don't like the requirement for a minimum length for the name string. I think we should only automate checking the existence of such a string, and leave the user to determine if it is human readable or not.
| def test_5src_check_value_name_not_long_enough(): | ||
| sparql = """ | ||
| PREFIX schema: <http://schema.org/> | ||
| PREFIX shp: <https://w3id.org/shp#> | ||
| PREFIX rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#> | ||
|
|
||
| DELETE { | ||
| ?this schema:name ?name . | ||
| } | ||
| INSERT { | ||
| ?this schema:name "Short" . | ||
| } | ||
| WHERE { | ||
| ?this schema:additionalType shp:CheckValue . | ||
| } | ||
| """ | ||
|
|
||
| do_entity_test( | ||
| rocrate_path=ValidROC().five_safes_crate_result, | ||
| requirement_severity=Severity.REQUIRED, | ||
| expected_validation_result=False, | ||
| expected_triggered_requirements=["CheckValue"], | ||
| expected_triggered_issues=["CheckValue MUST have a human readable name string of at least 20 characters."], | ||
| profile_identifier="five-safes-crate", | ||
| rocrate_entity_mod_sparql=sparql, | ||
| ) |
There was a problem hiding this comment.
I don't think we should have this test, as we shouldn't be defining how long the name string should be.
| ?this schema:name ?name . | ||
| } | ||
| INSERT { | ||
| ?this schema:name 123 . |
There was a problem hiding this comment.
This test is ambiguous, as we are not explicitly setting the data type here. Can we set the type of this object as well as providing a non-string value?
Alternatively, we replace it with a test which just checks for the existence of a name object.
| five-safes-crate:CheckValueObjectShouldPointToRootDataEntity | ||
| a sh:NodeShape ; | ||
| sh:name "CheckValue" ; | ||
| sh:description "" ; | ||
| sh:target [ | ||
| a sh:SPARQLTarget ; | ||
| sh:select """ | ||
| PREFIX schema: <http://schema.org/> | ||
| PREFIX shp: <https://w3id.org/shp#> | ||
| SELECT ?this | ||
| WHERE { | ||
| ?this schema:additionalType shp:CheckValue . | ||
| } | ||
| """ ; | ||
| ] ; | ||
|
|
||
| sh:property [ | ||
| a sh:PropertyShape ; | ||
| sh:name "object" ; | ||
| sh:path schema:object ; | ||
| sh:minCount 1 ; | ||
| sh:hasValue <./> ; | ||
| sh:severity sh:Warning ; | ||
| sh:message "`CheckValue` --> `object` SHOULD point to the root of the RO-Crate" ; | ||
| ] . |
There was a problem hiding this comment.
I think we should point to an rocrate:RootDataEntity here, rather than checking for an object that has a <./> value. Just in case the definition of the root data entity changes later.
See this code for how I did this for the Sign-Off phase:
This PR addresses #45
Implemented rules
RootDataEntity-->mentionsSHOULD reference theCheckValueentityCheckValueentity MUST haveadditionalTypewith@idofhttps://w3id.org/shp#CheckValue(i)CheckValueentity MUST be of typeAssessActionnameof theCheckValueentity MUST provide a human-readable summary of the review and result (ii)CheckValue-->objectSHOULD point to the root of the RO-Crate.CheckValue-->instrumentSHOULD point to an entity typedschema:DefinedTermCheckValueentity MAY havestartTimeproperty if action beganCheckValue-->startTimeMUST follows the RFC 3339 standard.CheckValueentity SHOULD haveendTimeproperty if action endedCheckValue-->endTimeMUST follows the RFC 3339 standard.CheckValueentity SHOULD haveactionStatuspropertyCheckValue-->actionStatusMUST have one of the allowed valuesCheckValue-->agentSHOULD reference the agent who initiated the check (iii)(i) Rule 2 is not an actual rule, but rather a definition of what constitutess a CheckPhase object. Hence it is not possible to code it, given its axiomatic nature.
(ii) This cannot be entirely done without a manual check. Here we only check that the
nameproperty of the Check Phase object is a string of at least 20 characters.(iii) All we are enforcing / testing here is that
checkValuehas a propertyagentwhich points to an IRI.Rules not implemented (they need some clarification)