Skip to content
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

Trouble with qualified value shape #213

Closed
ajnelson-nist opened this issue Oct 27, 2023 · 6 comments
Closed

Trouble with qualified value shape #213

ajnelson-nist opened this issue Oct 27, 2023 · 6 comments

Comments

@ajnelson-nist
Copy link
Contributor

Hello,

I'm having a difficult time making a qualified value shape work.

I've tried the graph ex:Hand example in SHACL Section 4.7.3, and have gotten it to trigger as the spec has me think it should, with this data:

@prefix ex: <http://example.com/ns#> .

ex:hand-1
	a ex:Hand ;
	ex:digit
		[ a ex:Finger ] ,
		[ a ex:Finger ] ,
		[ a ex:Finger ] ,
		[ a ex:Finger ] ,
		[ a ex:Finger ]
		;
	.

The results of running pyshacl --shacl HAND_SH.ttl HAND_DATA.ttl are:

Validation Report
Conforms: False
Results (2):
Constraint Violation in QualifiedValueShapeConstraintComponent (http://www.w3.org/ns/shacl#QualifiedMinCountConstraintComponent):
	Severity: sh:Violation
	Source Shape: [ sh:path ex:digit ; sh:qualifiedMaxCount Literal("1", datatype=xsd:integer) ; sh:qualifiedMinCount Literal("1", datatype=xsd:integer) ; sh:qualifiedValueShape [ sh:class ex:Thumb ] ]
	Focus Node: ex:hand-1
	Result Path: ex:digit
	Message: Focus node does not conform to shape MinCount 1 MaxCount 1: [ sh:class ex:Thumb ]
Constraint Violation in QualifiedValueShapeConstraintComponent (http://www.w3.org/ns/shacl#QualifiedMaxCountConstraintComponent):
	Severity: sh:Violation
	Source Shape: [ sh:path ex:digit ; sh:qualifiedMaxCount Literal("4", datatype=xsd:integer) ; sh:qualifiedMinCount Literal("4", datatype=xsd:integer) ; sh:qualifiedValueShape [ rdf:type sh:NodeShape ; sh:class ex:Finger ] ]
	Focus Node: ex:hand-1
	Result Path: ex:digit
	Message: Focus node does not conform to shape MinCount 4 MaxCount 4: [ rdf:type sh:NodeShape ; sh:class ex:Finger ]

(This worked with and without sh: qualifiedValueShapesDisjoint included)

But, I'm having a hard time getting a reduced example to trigger.

The shapes graph I'm trying is:

@prefix ex: <http://example.org/ontology/> .
@prefix rdfs: <http://www.w3.org/2000/01/rdf-schema#> .
@prefix sh: <http://www.w3.org/ns/shacl#> .

ex:MyShape
	a sh:NodeShape ;
	sh:targetClass ex:MyFirstClass ;
	sh:property [
		rdfs:comment "This triggers as expected."@en ;
		sh:path ex:myProperty ;
		sh:minCount 1 ;
	] ;
	sh:property [
		rdfs:comment "This does not trigger...why?"@en ;
		sh:path ex:myProperty ;
		sh:qualifiedValueShape [
			sh:class ex:MySecondClass
		] ;
		sh:qualifiedMinCount 1 ;
	]
	.

The data graph I'm feeding in is:

@prefix ex: <http://example.org/ontology/> .
@prefix kb: <http://example.org/kb/> .

kb:MyFirstClass-instance-not-ok
	a ex:MyFirstClass ;
	.

kb:MyFirstClass-instance-ok
	a ex:MyFirstClass ;
	ex:myProperty [
		a ex:MySecondClass ;
	] ;
	.

And this is the only result I'm getting from running pyshacl --metashacl --shacl POST_SH.ttl POST_DATA.ttl:

Validation Report
Conforms: False
Results (1):
Constraint Violation in MinCountConstraintComponent (http://www.w3.org/ns/shacl#MinCountConstraintComponent):
	Severity: sh:Violation
	Source Shape: [ rdfs:comment Literal("This triggers as expected.", lang=en) ; sh:minCount Literal("1", datatype=xsd:integer) ; sh:path ex:myProperty ]
	Focus Node: kb:MyFirstClass-instance-not-ok
	Result Path: ex:myProperty
	Message: Less than 1 values on kb:MyFirstClass-instance-not-ok->ex:myProperty

(--metashacl didn't raise any complaints.)

I was expecting a second sh:Violation to occur based on the qualified value shape. The first violation shows the unqualified minimum count is working. Are there any thoughts on why the qualified minimum count isn't working?

@ashleysommer
Copy link
Collaborator

@ajnelson-nist Thanks for reporting this. This is one of the areas where the SHACL W3C Test Suite (SHT) and the Datashape Tests suite (DASH) both are lacking comprehensive and exhaustive test cases.
This is likely a bug or oversight in implementation in PySHACL, so first I'll create a valid reproduction and see if I can track down the source of the issue.

@ashleysommer
Copy link
Collaborator

ashleysommer commented Nov 7, 2023

Ok, I've found the source of the problem. It is due to a potentially overzealous optimisation added when the feature to detect and prevent shape-recursion was implemented in Sept 2022.

# Shortcut, when there are no value nodes, don't check for recursion, don't validate and exit early
value_node_count = 0
for f, value_nodes in focus_value_nodes.items():
value_node_count = value_node_count + len(value_nodes)
if value_node_count < 1:
return (not non_conformant), reports

Basically, the second PropertyShape on MyShape does not see any focus/value nodes at the path sh:path ex:myProperty on kb:MyFirstClass-instance-not-ok so it skips checking the Constraints on that path. Thats why the violation of that constraint does not trigger.

So I suppose an easy solution would be to skip this shortcut check if qualifiedMinCount > 0 , so that it triggers on the absence of values nodes.
It does raise another question though: Is the complete absence of value nodes on that property path considered logically the same as less-than-one 'qualified' nodes on that path that are conformant to the qualifiedValueShape?
It does hold true for sh:minCount as you pointed out, so indeed it probably should also work for qualifiedMinCount on qualifiedValueShape.

@ajnelson-nist
Copy link
Contributor Author

So I suppose an easy solution would be to skip this shortcut check if qualifiedMinCount > 0 , so that it triggers on the absence of values nodes.

I think this would be correct.

It does raise another question though: Is the complete absence of value nodes on that property path considered logically the same as less-than-one 'qualified' nodes on that path that are conformant to the qualifiedValueShape? It does hold true for sh:minCount as you pointed out, so indeed it probably should also work for qualifiedMinCount on qualifiedValueShape.

Yes, I believe the complete absence of value nodes equally effects unqualified property shapes and qualified property shapes. Looking over the qualified shapes part of the spec, the phrasing does not seem to have a "qualified matches among matches found" logical flex. Instead, sentence one looks pretty definitive: "sh:qualifiedValueShape specifies the condition that a specified number of value nodes conforms to the given shape."

@ashleysommer
Copy link
Collaborator

ashleysommer commented Nov 7, 2023

Yes, I agree with your assessment, and that's the same conclusion I came to.
I've pushed a fix for this d939e30
It will be doing the big churn to support RDFLib 7.0.0 this weekend, then the new version will be pushed out, including this fix.
While debugging this, I also found an edge-case error in my implementation of handling sh:qualifiedValueShapesDisjoint so that's a bonus.

@ajnelson-nist
Copy link
Contributor Author

Spectacular to hear! Many thanks!

@ashleysommer
Copy link
Collaborator

Okay, @ajnelson-nist you don't need to wait until the weekend. The new version is out already.

ajnelson-nist added a commit to casework/CASE-Corpora that referenced this issue Nov 9, 2023
A local-to-CASE-Corpora shape requires `InvestigativeAction`s to have
`ProvenanceRecord`s as outputs.  (See `/shapes/local.ttl`,
`sh-case-corpora-local:InvestigativeAction-shape`.)
That shape used a SHACL mechanism that was not active in this
repository's testing until pySHACL Issue 213 was closed.

This patch adds a `ProvenanceRecord` as output to the last non-subaction
that had the phone as input.  Review of the results of this SPARQL query
indicate the device is not used in further actions, so this
`ProvenanceRecord` has no further impact on the graph today.

```sparql
SELECT ?nAction ?lDescription
WHERE {
  ?nAction
    uco-action:object kb:device-ea732801-7d0e-46ac-a028-69b782c97a46 ;
    .
  OPTIONAL {
    ?nAction
      uco-core:description ?lDescription ;
      .
  }
}
ORDER BY ?nAction
```

There is some open question on how to tie the subactions' outputs to the
parent action's `ProvenanceRecord`; that thread is on CASE Issue 136.

A follow-on patch will regenerate Make-managed files.

References:
* RDFLib/pySHACL#213
* casework/CASE#136

Signed-off-by: Alex Nelson <alexander.nelson@nist.gov>
ajnelson-nist added a commit to casework/CASE-Utilities-Python that referenced this issue Nov 13, 2023
The `case_validate` version is bumped to acknowledge behavior changes
from pySHACL 0.24.0.

References:
* RDFLib/pySHACL#213

Signed-off-by: Alex Nelson <alexander.nelson@nist.gov>
ajnelson-nist added a commit to casework/CASE-Corpora that referenced this issue Nov 15, 2023
This patch re-introduces `DownloadableFile`, as well as some related
classes, to describe a concept IRI that behaves as a file-yielding URL.

This patch removes a qualified SHACL constraint that had been written on
a conflated-classes guess that instigated UCO Issue 534.  After
discussion on Issue 534, this shape should have been removed, but it
wasn't until pySHACL Issue 213 was addressed that the shape started
triggering and raising awareness it was still around.

Hand-maintained data, as well as the Digital Corpora rendering script,
have been updated to accommodate the new shapes around
`DownloadableObject` and `DownloadableRelation`.  A later patch will
need to add the `DownloadableRelation`s to the chain of custody.

A follow-on patch will regenerate Make-managed files.

References:
* RDFLib/pySHACL#213
* ucoProject/UCO#534

Signed-off-by: Alex Nelson <alexander.nelson@nist.gov>
ajnelson-nist added a commit to casework/CASE-Corpora that referenced this issue Nov 15, 2023
This patch also updates a test result expectation to account for an
effect of pySHACL 0.24.0.

A follow-on patch will regenerate Make-managed files.

References:
* RDFLib/pySHACL#213

Signed-off-by: Alex Nelson <alexander.nelson@nist.gov>
ajnelson-nist added a commit to casework/CASE-Corpora that referenced this issue Nov 15, 2023
References:
* RDFLib/pySHACL#213

Signed-off-by: Alex Nelson <alexander.nelson@nist.gov>
ajnelson-nist added a commit to casework/CASE that referenced this issue Jan 23, 2024
This new shape stemmed from discussion on CASE Issue 136.

As a matter of preserving backwards compatibility, this patch introduces
the shape requiring `ProvenanceRecord`s with a `sh:Warning`-level
severity.  In CASE 2.0.0, this requirement will be strengthened into a
`sh:Violation`.

A separate proposal will be filed with UCO to test the minimum qualified
cardinality OWL structure.  A draft of that syntax review system was
used to test this patch.

This patch adds a version floor for pySHACL to ensure an update in
qualified value shape handling is included, which is necessary for the
new property shape to function when using pySHACL.

Disclaimer:

References:
* RDFLib/pySHACL#213
* #136
* #146

Signed-off-by: Alex Nelson <alexander.nelson@nist.gov>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants