lambdamusic / Ontospy

Python library and command-line interface for inspecting and visualizing RDF models aka ontologies.
http://lambdamusic.github.io/Ontospy/
MIT License
219 stars 52 forks source link

Additional SHACL support to Ontospy/gendocs #107

Closed balon closed 2 years ago

balon commented 2 years ago

This patch series is intended to bring Ontospy's current SHACL support up to the level of displaying Property Shapes on class pages and Shape Properties on shape pages.

A new module, _ontospy/core/shaclhelper.py, returns SHACL property constraints, is attached to each OntoClass object, once the shapes are built in Ontospy.

A few notes on changes:

Please advise if there is a better approach to increasing the support for SHACL into Ontospy, or if any modifications are needed to the current patch presented.

lambdamusic commented 2 years ago

Thanks. I'll be able to review it over the next few days. If you could attach/reference a sample rdf file for testing all new features that'd be great.

balon commented 2 years ago

Attached is uco_monolithic.ttl, which can be used for testing the features in this patch series. The file could also be constructed by running the makefile found in the ucoProject/UCO repository.

Thanks!

uco_monolithic.ttl.zip

ajnelson-nist commented 2 years ago

This PR has received an in-flight update, for the same reason noted over on PR 108.

lambdamusic commented 2 years ago

First of all, thanks! This looks like a terrific addition to Ontospy.

I just released a minor new version v 1.9.9.4 that should simplify merging this functionality (it's in master), so would you please update your changes accordingly?

Here's a few comments to get started.

  1. I am not too familiar with Shacl patterns - but I do understand that it is customary to add two types to the same URI (a class and a shape). Ontospy always took the naive approach of a unique-URI-per-entity-type, so I can see why you changed this line (..if not test_existing_cl:.... That shouldn't be necessary anymore with the just released v 1.9.9.4, I've added a more specific method self.get_shapes that allows to safely index classes/shapes with the same URI.

  2. For some reason the documentation generation is backward incompatible. See for example this documentation page for SciGraph - RDF source here - previously I was able to generate a 'Shapes' page for SciGraph, but with your patch I get none. Any idea why?

  3. More generally, re the templates: I don't fully understand how/where you would like to organise the information about Shapes in the HTML templates .. ideally for me any changes should be backward compatible. I'll be spending more time on this after points 1 and 2 above are sorted.

Hope this makes sense..

PS Just FYI I was planning to clean up the whole codebase (admittedly a bit dated and messy!) and release a V2 of Ontospy in the near future, with better docs etc.... I'll hold off doing that until we complete this PR!

tobiasschweizer commented 2 years ago

Hi

I added a feature request in #104 and I think your PR does what I strive for :-)

I did the following:

What I get as HTML looks correct in terms of property shapes on the class page. This is really nice!

However, I tried this with my own ontology which consists of

As observed in https://github.com/lambdamusic/Ontospy/pull/107#issuecomment-997284164 I don't see the Shapes page anymore.

Could this be because classes and node shapes do not use the same IRI? For some more details about the ontology I use, please see #104

Thanks a lot!

ajnelson-nist commented 2 years ago

Thank you both for the feedback! We'll be getting back to you next week.

balon commented 2 years ago

Thank you for the feedback. We had a discussion today about a timeline to updating the PR as requested. We're both currently tied up on a few tasks, but plan to address this early next week with plans to have something ready to push by next Friday.

We will also look at the issues presented involving the missing shapes page on some ontologies.

lambdamusic commented 2 years ago

Sounds good, thanks. If there are any updates at my end, I'll let you know.

ajnelson-nist commented 2 years ago

We're working through the remarks now. We've made a catch-up merge to integrate master's current state, and have converted the branch to a draft state while we step through the other remarks.

@lambdamusic , re:

For some reason the documentation generation is backward incompatible. See for example this documentation page for SciGraph - RDF source here - previously I was able to generate a 'Shapes' page for SciGraph, but with your patch I get none. Any idea why?

Could you explain what behavior you're seeing here? After our catch-up merge, we regenerated the page, and saw these effects:

I suspect we've actually resolved your second issue, but that's mainly from our failure to reproduce what you saw.

ajnelson-nist commented 2 years ago

On further review, sh:predicate is not defined in SHACL. Did the scigraph RDF source intend to use sh:path?

ajnelson-nist commented 2 years ago

On the shapes page, we intend to list a table of how properties associate with a class. There's a conceptual hop there, which is probably something we need to talk through to come to a good design consensus.

The user interface complexity we're trying to hide is going to apply to SHACL-based schemes that don't define owl:Restrictions. In our use case, we don't have owl:Restrictions, but we do have sh:PropertyShapes. So, our property associations would be listed on the Shapes HTML page. But, to see things about other classes in the parent/child hierarchy, the user would hop over to the Class page, where no properties would be listed.

So, we tried to apply some redundancy - having the properties associated via SHACL be listed on the class page as well. This saves the user two clicks (oscillating between shapes and class pages) per level of the class hierarchy they want to review.

We probably wouldn't go for this redundancy if SHACL had a concept of "SubNodeShape," but they don't.

That's the feedback we could get to today, we're happy to discuss this further.

lambdamusic commented 2 years ago

Hi folks - first of all, wish I had time to get back to you sooner, sorry about that and thanks a lot for the patience. I reviewed the PO after the merging, have three main comments.

1) Shapes HTML page for scigraph is ok

@ajnelson-nist as you pointed out, the scigraph ontology 'shapes' page is not missing anymore. Don't know what's changed, I do recall noticing the difference, but of course I might be wrong. So all's good there. Backward compatibility is fine.

2) New ontospy tests error

I do get an error when running the test suite (./tools/run-all-tests.sh). The error is triggered when trying to scan the schema test RDF model. You can reproduce simply by typing $ ontospy scan ontospy/tests/rdf/schema/ (on this branch, of course). Here's the traceback:

...
  File "/Users/michele.pasin/code/python/ontospy/ontospyProject/ontospy/cli.py", line 181, in scan
    action_analyze(sources, endpoint, print_opts, verbose, extra, raw, individuals, format)
  File "/Users/michele.pasin/code/python/ontospy/ontospyProject/ontospy/core/actions.py", line 123, in action_analyze
    g = Ontospy(
  File "/Users/michele.pasin/code/python/ontospy/ontospyProject/ontospy/core/ontospy.py", line 134, in __init__
    self.build_all(
  File "/Users/michele.pasin/code/python/ontospy/ontospyProject/ontospy/core/ontospy.py", line 249, in build_all
    self.build_shapes()
  File "/Users/michele.pasin/code/python/ontospy/ontospyProject/ontospy/core/ontospy.py", line 635, in build_shapes
    shacl_constraints = build_shacl_constraints(self)
  File "/Users/michele.pasin/code/python/ontospy/ontospyProject/ontospy/core/shacl_helper.py", line 255, in build_shacl_constraints
    add_property_constraints_from_shape_triples(lineage_class.uri, property_constraints, shape.triples)
  File "/Users/michele.pasin/code/python/ontospy/ontospyProject/ontospy/core/shacl_helper.py", line 298, in add_property_constraints_from_shape_triples
    raise Exception('Something is very wrong.  Class URI not in triples.')
Exception: Something is very wrong.  Class URI not in triples.

The error is triggered by line line 297. Not really sure I fully understand the logic there, so maybe you can shed some light.

Depending on how bad the cause of the error is, I'd suggest issuing a warning instead and/or failing silently. Ontospy is not really a schema validator, so I'd prefer to render an incomplete output rather than failing completely.

3) SciGraph shapes error

You are totally right @ajnelson-nist, sh:predicate should be sh:path. Not sure what happened there, but I suppose when we built that model a few years back we may have used an older Shacl specification which is now deprecated. Who knows..

Anyhow, good that you spotted it. I've updated the test RDF with sh:path within a new folder scigraph2 and pushed it to this branch. Tried to run ontospy on it, but also here I'm getting the error above:

  File "/Users/michele.pasin/code/python/ontospy/ontospyProject/ontospy/core/shacl_helper.py", line 298, in add_property_constraints_from_shape_triples
    raise Exception('Something is very wrong.  Class URI not in triples.')
Exception: Something is very wrong.  Class URI not in triples.

So, even more curious about how to get rid of that.


To sum up - I believe that after addressing that validation error above, we're good to go :-)

The UI issues you mentioned are important, but maybe better tackled outside this PR and by using more test input data.

ajnelson-nist commented 2 years ago

First, @tobiasschweizer , you were right to give a poke, thanks for that.

@lambdamusic , thank you for the test case. We stumbled on a similar issue. @balon and I have started a type review of the shacl_helper.py script on observing that there were a few errant assumptions about nodes being required to be blank nodes or rdflib.URIRefs.

ajnelson-nist commented 2 years ago

@lambdamusic : @balon and I think the "Something is very wrong" exception has been addressed. I think we've covered the tests you've mentioned above; ./tools/run-all-tests.sh and nosetests both pass. If there's nothing else, this might be good to merge.

lambdamusic commented 2 years ago

Thanks @ajnelson-nist @balon this is good to go!

lambdamusic commented 2 years ago

Available in https://github.com/lambdamusic/Ontospy/releases/tag/2.0.0-alpha

ajnelson-nist commented 2 years ago

Thank you!

tobiasschweizer commented 2 years ago

Hi there,

That is great news, I'll try this asap.