w3c / data-shapes

RDF Data Shapes WG repo
87 stars 33 forks source link

optional values not handled correctly in test cases process #59

Closed pfps closed 7 years ago

pfps commented 7 years ago

The test cases process in SHACL Test Suite and Implementation Report W3C Document 19 April 2017 at http://w3c.github.io/data-shapes/data-shapes-test-suite/ does not correctly account for some optional aspects of validation results. Both sh:sourceShape and sh:sourceConstraint are optional in some validation results. This produces different acceptable validation results that are non-isomorphic.

These aspects of validation results cannot just be excluded from the checking process because there are requirements on the values if present.

HolgerKnublauch commented 7 years ago

What do you suggest instead?

pfps commented 7 years ago

I suggest modifying the test suite process so that such triples are considered optional in validation results, perhaps by including values for sht:optional which are properties whose values are considered optional. These triples would be removed before comparison, which would itself need to be augmented to allow for the optionality. Another way would be to split the optional triple in two, something like validationresult sht:optional [ sh:sourceShape value ].

HolgerKnublauch commented 7 years ago

So far all three submitted test reports had no issue with this. Are you planning to submit a test report and are you affected by this? I am afraid that while we could indeed add such a mechanism somehow, it is just adding (unnecessary) work for everyone. The primary role of this test suite document is to serve as an intermediate step during the W3C process. It is not a document that is supposed to cover all eventualities and theoretical possibilities. Neither is it a shadow spec. So unless any implementor wants to submit results but is unable to do so because of the inability to produce these triples, I see no need to make this change at the current time.

pfps commented 7 years ago

Here is an example validation report from my implementation. It doesn't include the sh:sourceShape triples and thus would be counted as a failure. @prefix rdf: http://www.w3.org/1999/02/22-rdf-syntax-ns# . @prefix rdfs: http://www.w3.org/2000/01/rdf-schema# . @prefix sh: http://www.w3.org/ns/shacl# . @prefix xml: http://www.w3.org/XML/1998/namespace . @prefix xs: http://www.w3.org/2001/XMLSchema# . @prefix xsd: http://www.w3.org/2001/XMLSchema# .

http://nuance.com/shacl#ValidationResult rdfs:subClassOf sh:ValidationResult .

[] a sh:ValidationReport ; sh:conforms false ; sh:result [ a http://nuance.com/shacl#ValidationResult ; sh:focusNode "2017-03-29"^^xs:date ; sh:parameter 4 ; sh:sourceConstraintComponent sh:MaxLengthConstraintComponent ; sh:value "2017-03-29"^^xs:date ], [ a http://nuance.com/shacl#ValidationResult ; sh:focusNode http://datashapes.org/sh/tests/core/node/maxLength-001.test#John ; sh:parameter 4 ; sh:sourceConstraintComponent sh:MaxLengthConstraintComponent ; sh:value http://datashapes.org/sh/tests/core/node/maxLength-001.test#John ], [ a http://nuance.com/shacl#ValidationResult ; sh:focusNode "Hello" ; sh:parameter 4 ; sh:sourceConstraintComponent sh:MaxLengthConstraintComponent ; sh:value "Hello" ], [ a http://nuance.com/shacl#ValidationResult ; sh:focusNode 12345 ; sh:parameter 4 ; sh:sourceConstraintComponent sh:MaxLengthConstraintComponent ; sh:value 12345 ], [ a http://nuance.com/shacl#ValidationResult ; sh:focusNode :f2920c64a659c4f6c997ba3295d34b5b7b2 ; sh:parameter 4 ; sh:sourceConstraintComponent sh:MaxLengthConstraintComponent ; sh:value :f2920c64a659c4f6c997ba3295d34b5b7b2 ] .

HolgerKnublauch commented 7 years ago

So are you planning to submit an implementation report? This would be good news. The last thing we heard from you was the notice of non-implementation: https://lists.w3.org/Archives/Public/public-rdf-shapes/2017Feb/0143.html If yes, would this cover SHACL-SPARQL?

On the absence of sh:sourceShape, couldn't we simply say "Implementations that do not produce sh:sourceShape triples must delete them from the "expected" results graph prior to the graph isomorphism comparison ?

pfps commented 7 years ago

I have been modifying my implementation of SHACL to conform to the current definition. Several of the recent changes to SHACL have made that much more needlessly difficult. The continuing ambiguities and problems in SHACL have also contributed to the increase in difficulty. I have also been spending considerable time on the problems with EXISTS and pre-binding.

My implementation has never included a version of SHACL-SPARQL based on pre-binding, so, no, I will not be producing an implementation of SHACL-SPARQL as currently defined. My SPARQL extension used a very different mechanism.

I am having doubts as to whether it it worthwhile to finish my modifications given the current syntax of SHACL and its severe interoperability problems. I will certainly not be recommending use of SHACL in its current state within Nuance.

pfps commented 7 years ago

Implementations may produce values for sh:sourceShape only sometimes. An option to the test harness to remove sh:sourceShape values form the "expected" result has to take this into account.

HolgerKnublauch commented 7 years ago

Ok thanks for letting us know. In practice this indicates to me that no further changes to the test framework are on a critical path. No other implementation has requested them.

pfps commented 7 years ago

So the only thing that should be done to the test cases document is to note that the check is incomplete (and wrong for sh:message).

HolgerKnublauch commented 7 years ago

See my commit above, I have added such a general statement so that people can fall back to manually checking results. Anything else needed here?

pfps commented 7 years ago

No longer worth worrying about at this time.