shaclgen: Add --include-annotations option to let annotations be part of shacl shapes

[jsheunis]

This is for the SHACL generator in response to #1618.

Code is added to shaclgen.py to:

• allow users to specify the --include-annotations tag if they want annotations (on classes, slots, and types) to be included in the exported SHACL shapes
• determine the datatype of both annotation tag and value:
  • a CURIE is identified by searching for the ':' character in the tag or value
  • if not a CURIE, the annotation tag is assumed to be a literal with type string
  • if not a CURIE, the annotation value is a literal with datatype determined using python type and extended_float, extended_int, extended_str, etc
  • add the correct triples to the shacl output (to a nodeshape for classes, and to a property shape for slots and slots with types as ranges)

In addition, the test_shaclgen test is updated to test for annotations on classes, and the kitchen sink schema is updated to include annotations on the person class. Several snapshot data files needed to be updated as well because of the change to that schema, so that existing tests would still succeed.

Uncertainties:

• I was unsure how to test for annotations on types and slots, because the related rdf terms in the graph generated from the kitchen sink schema would be blank nodes with transient IDs, so these can't be captured in the "EXPECTED" triples.
• Are there other elements that can also have annotations that aren't included and should ideally form part of this PR?
• I am not sure if my approach to get the element type, and hence the XSD datatype, is sensible; happy to update this approach if there are LinkML-preferred ways of doing the same
• I currently have a workaround in the code for a jsonasobj2 issue, mentioned here: jsonasobj2 causes inconsistent behavior on attr assignment vs. instantiation for linkml_model classes #1665 (comment)
• UPDATE: conflicts can arise when a slot has annotations, and if that same slot has a type as a range, where the type also has annotations. If some of these annotations have the same tag, the current way that the code is implemented means that the values will be appended, not replaced. It seems to me that the desired behaviour will differ based on the use case. So the question is whether more user-defined controls should be implemented to direct such behaviour?

[codecov]

Codecov Report

Attention: Patch coverage is 78.94737% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 79.86%. Comparing base (c5b0542) to head (bc364b2).
Report is 1 commits behind head on main.

Files	Patch %	Lines
linkml/generators/shaclgen.py	78.94%	5 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2111      +/-   ##
==========================================
- Coverage   79.86%   79.86%   -0.01%     
==========================================
  Files         110      110              
  Lines       12318    12355      +37     
  Branches     3507     3520      +13     
==========================================
+ Hits         9838     9867      +29     
- Misses       1884     1889       +5     
- Partials      596      599       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[cmungall]

This could be moved into schemaview (but doesn't need to happen for this PR)

[jsheunis]

Regarding:

• UPDATE: conflicts can arise when a slot has annotations, and if that same slot has a type as a range, where the type also has annotations. If some of these annotations have the same tag, the current way that the code is implemented means that the values will be appended, not replaced. It seems to me that the desired behaviour will differ based on the use case. So the question is whether more user-defined controls should be implemented to direct such behaviour?

I just came across a relevant example. Among other annotations, I want to assign sh:order to slots to define how they are represented in a user interface autogenerated from the exported SHACL. E.g.:

slot_usage:   
   distance:
      annotations:
         sh:group: mycurie:BasicPropertyGroup
         sh:order: 0

the output from gen-shacl will then be:

[ sh:datatype xsd:string ;
     sh:description "Distance of a thing from source location." ;
     sh:group mycurie:BasicPropertyGroup ;
     sh:nodeKind sh:Literal ;
     sh:order 0,
        27 ;
     sh:path mycurie:distance ],

i.e. the sh:order has two comma-separated values. This is because the code also assigns order to each slot that is being processed:

linkml/linkml/generators/shaclgen.py

Lines 106 to 107 in a14e6ee

	prop_pv_literal(SH.order, order)
	order += 1

So, for my use case, I would want my annotation to overwrite whatever the code does internally.

I have solved this locally with the following patch:

diff --git a/linkml/generators/shaclgen.py b/linkml/generators/shaclgen.py
index dc804cb5..729d80f6 100644
--- a/linkml/generators/shaclgen.py
+++ b/linkml/generators/shaclgen.py
@@ -109,8 +109,6 @@ class ShaclGenerator(Generator):
                         g.add((pnode, p, Literal(v)))

                 prop_pv(SH.path, slot_uri)
-                prop_pv_literal(SH.order, order)
-                order += 1
                 prop_pv_literal(SH.name, s.title)
                 prop_pv_literal(SH.description, s.description)
                 if not s.multivalued:
@@ -185,6 +183,10 @@ class ShaclGenerator(Generator):
                     if s.annotations and self.include_annotations:
                         self._add_annotations(prop_pv, s)

+                if (pnode, SH.order, None) not in g:
+                    prop_pv_literal(SH.order, order)
+                    order += 1
+
                 ifabsent_processor.process_slot(prop_pv, s, class_uri)

         return g

[cmungall]

Thank you!

yes, a limit of the current rdf testing is that it's incomplete when blank nodes are present.

Override is the correct behavior in the case of clashes. We'll want to implement this same behavior in other rdf-serializing generators.

It would be good to extend test_annotation in test_annotation_compliance. This wouldn't add anything that is not already in your test, but the compliance testing framework is becoming the de facto standard way to test one construct across multiple generators. I am happy for this to be done in a later PR though

[jsheunis]

Does it make sense to make the SH.order-related patch highlighted above part if the PR as well? For me it does, but for my purposes I could also just apply the patch separately whenever I need to use shaclgen for that specific use case.

It would be good to extend test_annotation in test_annotation_compliance. This wouldn't add anything that is not already in your test, but the compliance testing framework is becoming the de facto standard way to test one construct across multiple generators. I am happy for this to be done in a later PR though

I can take a shot at this in a next PR 👍