w3c / data-shapes

RDF Data Shapes WG repo
87 stars 33 forks source link

Inconsistent sh:value for sh:hasValue #111

Closed langsamu closed 5 years ago

langsamu commented 5 years ago

In the test suite, the report for a hasValue constraint in a node shape includes sh:value: https://github.com/w3c/data-shapes/blob/6d811eab25d7c4e3c90382f76b5f38c624f0a230/data-shapes-test-suite/tests/core/node/hasValue-001.ttl#L34-L41

but the report for the same constraint in a property shape doesn't: https://github.com/w3c/data-shapes/blob/6d811eab25d7c4e3c90382f76b5f38c624f0a230/data-shapes-test-suite/tests/core/property/hasValue-001.ttl#L58-L65

The spec for 4.8.2 sh:hasValue only says that there is a validation result. It doesn't mention sh:value.

Is this a mistake or a significant difference?

HolgerKnublauch commented 5 years ago

I believe you have discovered a buglet in the spec or the test suite. The spec states that no sh:value should be generated (by not mentioning sh:value in the definition of sh:hasValue). The test suite is inconsistent. Possible actions: either we change the spec to always generate the sh:value or we remove the sh:value from the node shape test case. I have no strong preference. Arguably, sh:value with an actual value would be helpful to users, yet it would point at a triple that doesn't exist in the data, and all relevant info can theoretically be derived from the other attributes of the validation result.

At this stage I guess we need to keep this ticket open until there is consensus on how to resolve it. Given that we are outside of the WG's life cycle, I believe changing the test suite is "cheaper" than changing the spec, but I am not an expert in W3C processes.

For now I have marked the offending test case to link to this ticket:

https://github.com/w3c/data-shapes/commit/ce947a91386d2b03e12377935434e05d22131d29

Meanwhile, I welcome discussion on what solution we should apply, with thanks for opening this ticket.

TallTed commented 5 years ago

For certain, changing the test suite (which did not go through, but rather supported, the CR/PR/TR process, and is not part of but is meant to test compliance with the spec) is cheaper, and is the "correct" thing to do, unless and until it is demonstrated that this aspect of the spec is problematic to implementation and use, in which case we might have a spec erratum, which would be a factor in any future SHACL revision (whether 1.1, 2.0, or otherwise), and the test should then be modified to indicate both the "correct per TR" result and the "correct after errata are considered" result.

langsamu commented 5 years ago

From the limited perspective of recently implementing a SHACL processor, I don't find this aspect problematic and would prefer a change to the test case.

HolgerKnublauch commented 5 years ago

Agreed. Test case changed. Will also send an email to SHACL CG to inform implementers.

https://github.com/w3c/data-shapes/commit/b5f4d0c8a3722ea20407ab9a2ac6a45fddfd7c09