-
Couldn't load subscription status.
- Fork 37
added sh:conformanceDisallows, updated conformance-definition #555
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
base: gh-pages
Are you sure you want to change the base?
Conversation
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.
Would it make sense to add some test cases as well?
|
@bergos I searched for "disallowed" in your rendered version, and I don't understand something: UPD: I've read #453 and in particular #453 (comment) (which is the basis of the voting). You say at the start:
|
|
@VladimirAlexiev You are right. I revisited severity section and added a note on how the conformance disallow set is defined. |
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 only have one blocking remark on a paragraph about engines interacting with defaults. Either it's a group-discussion matter for a significant behavior in engines providing defaults, or there's just a little English cleanup to clarify that engines can provide some way for users to request different conformance levels for a single validation run.
|
@bergos I think it's still backwards:
|
Co-authored-by: Alex Nelson <[email protected]>
@ajnelson-nist That would only work if we make |
@VladimirAlexiev The set in the validation report is a representation of the set of the engine. I think that's an implementation detail. One could write an implementation that creates the validation report before the validation and uses that to store the set. I think the spec should not cover how an engine keeps intermediate states/values and how they are copied during processing. |
Co-authored-by: Alex Nelson <[email protected]>
|
@ajnelson-nist I've merged the changes you proposed. |
Test contents adapted from Core document example by @bergos. Test structure adapted from this file, picked more or less at random: shacl12-test-suite/tests/core/path/path-alternative-001.ttl Signed-off-by: Alex Nelson <[email protected]>
Added in a43a352 , thank you for the reminder. |
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.
Someone else should approve before merge, if not to review the text, then to check the tests I just added.
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.
The ex:PersonShape-age shape is used in both validation reports, once with severity sh:Warning and once with severity sh:Violation.
This shape is later defined in the specification as follows:
ex:PersonShape-age;
a sh:PropertyShape;
sh:path ex:age;
sh:datatype xsd:integer;
sh:maxCount 1;
sh:reifierShape ex:ProvenanceShape;
sh:reificationRequired true .If the data graph in Example 24 is the one used for illustration, I think the validation reports are missing the reification validation results.
Ah, whoops. Good catch, I forgot to search for the shape definition. One response pertinent to the test design: I did keep the shape I wrote consistent between both tests. What I varied in the test was the presence of 28c28
< <conformance-disallows-001>
---
> <conformance-disallows-002>
31c31
< <conformance-disallows-001>
---
> <conformance-disallows-002>
33,35c33,35
< rdfs:label "Test of path sh:conformanceDisallows 001" ;
< rdfs:comment "conformanceDisallows presence lets sh:conforms be true despite presence of an sh:Warning-severity result. Test contents and behavior otherwise match conformance-disallows-002."@en ;
< rdfs:seeAlso <conformance-disallows-002> ;
---
> rdfs:label "Test of path sh:conformanceDisallows 002" ;
> rdfs:comment "conformanceDisallows absence causes sh:conforms be false due to presence of an sh:Warning-severity result. Test contents and behavior otherwise match conformance-disallows-001."@en ;
> rdfs:seeAlso <conformance-disallows-001> ;
42,43c42
< sh:conforms "true"^^xsd:boolean ;
< sh:conformanceDisallows sh:Violation ;
---
> sh:conforms "false"^^xsd:boolean ;Next step: The test discrepancy @YoucTagh noted seems to bubble up to the example that I copied. Is it a problem in the document to have two tests in unrelated sections use the same example IRI with different implementations around the example IRI? I'm fine with adjusting the test, but do we also need to adjust the document? |
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
|
@ajnelson-nist, @bergos Could we merge this pull request and create a new issue to discuss:
|
|
@ajnelson-nist I've merged the changes you proposed.
I don't think it's a problem to reuse IRIs for different examples. I didn't check, but I guess we already have some cases where that is done. |
|
@YoucTagh, please approve the PR, if you think it can be merged. |
|
@bergos, currently, the first thing a user sees in the validation report section is Examples 20 and 21. In these examples, we immediately present the validation report, which indicates whether the data graph conforms to the SHACL shape graph. The report includes the conformance result (true or false) and the overall validation outcome. However, when someone unfamiliar with the specification reviews this section for the first time, they will notice that the focus node is If you still don't think this is problematic, I won't block the merge. |
This PR adds the
sh:conformanceDisallowsproperty and updates the definition of the conformance check. The examples are updated to cover the non-default severity levels. The section for thesh:conformsproperty contained a redundant definition of the conformance check that was removed.Closes #453