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

SH_targetClass does not handle transitive rdfs:subClassOf #96

Closed
gtfierro opened this issue Oct 6, 2021 · 2 comments
Closed

SH_targetClass does not handle transitive rdfs:subClassOf #96

gtfierro opened this issue Oct 6, 2021 · 2 comments

Comments

@gtfierro
Copy link
Contributor

gtfierro commented Oct 6, 2021

Hi @ashleysommer! Hope all is well with you

I found what I believe is a similar issue to #87, this time around the handling of sh:targetClass. Below is a relatively minimal repro:

from pyshacl import validate

mixed_file_text = """
@prefix sh: <http://www.w3.org/ns/shacl#> .
@prefix rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#> .
@prefix owl: <http://www.w3.org/2002/07/owl#> .
@prefix rdfs: <http://www.w3.org/2000/01/rdf-schema#> .
@prefix : <urn:ex#> .

:Class0 a owl:Class .

:Class1 a owl:Class ;
    rdfs:subClassOf :Class0 .

:Class2 a owl:Class ;
    rdfs:subClassOf :Class1 .

:Class3 a owl:Class ;
    rdfs:subClassOf :Class2 .

:prop  a   owl:DatatypeProperty .

:shape a sh:NodeShape ;
    sh:targetClass :Class0 ;
    sh:property [
        sh:path :prop ;
        sh:hasValue "test" ;
        sh:minCount 1 ;
    ] .

:s2 a :Class2 ;
    :prop "fail" .

:s3 a :Class3 ;
    :prop "fail" .
"""

def test_fail():
    res1 = validate(mixed_file_text, data_graph_format='turtle', shacl_graph_format='turtle', debug=True)
    conforms, _, _ = res1
    assert not conforms

per this file, I would expect the validation to fail because :s2 and :s3 do not have the required "test" value on the property :prop. However, in the current handling of sh:targetClass, they are not targeted by the :shape NodeShape and thus do not fail the test. I expect this because of the SPARQL definition given in the SHACL standard.

I think the fix is fairly straightforward and I will submit a PR soon

@ashleysommer
Copy link
Collaborator

ashleysommer commented Oct 6, 2021

Hi @gtfierro
Thanks for opening this Issue and PR.

I believe, similar to the case in #87, I limited this to just two graph lookups (ex:focus rdf:type ex:target and ex:focus rdfs:subcClassOf ex:target) for performance reasons, and because that logic passes all SHACL test suite tests. We also assumed that the vast majority of users would use inference="rdfs" to perform graph expansion prior to validating, so all of the class memberships would be filled in anyway, so no further traversal would be necessary.

But you're right, the shacl spec does specify:

the set of SHACL instances of c in a data graph DG is a target

Where "SHACL instances" is the same definition we discussed in #87, so all subClassOf relationships need to be walked.

@ashleysommer
Copy link
Collaborator

Fixed by #97

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