Issue 669 2

[tfrancart]

(depends on other PR "issue-669-1" to work properly)
I realize this may not be the correct way to proceed, since the commits already in issue-669-1 are also in this branch. let me know.

[osma]

What's in this PR that's not in #670?
I think #670 should be sorted out and merged first.

Note that the Travis build failed for this PR but not for #670.

[tfrancart]

Thanks.
#670 only contains modifications in GenericSparql. This one modifies also Concept.php based on updates in GenericSparql.
Using the getLabel method in GenericSparql imposes a new constraint : the declared properties must have an rdf:type declared (because the SPARQL query imposes that). An extra rdf:type needs to be added in the test cases data. That's why some tests fail.
I will propose a fix on the test data.
I don't think this is problematic; do you have an opinion on this ?

[tfrancart]

Hi @osma if you had time to review and merge this in the next couple of weeks that would be great. We are targeting the deployment of Skosmos to publish legal vocabularies in early january, and having this improvement in the master branch would help.
Let me know.
Thanks

[osma]

We tested this with @henriyli and found out this imposes further constraints in addition to the rdf:type requirement you mention: the property definisions must be available in the default SPARQL endpoint, and union default graph mode needs to be enabled. If there is a property definition within the vocabulary, it won't be used if it's not available through the default graph of the default endpoint. So far Skosmos has not required a union default graph, and the current InstallTutorial section on Fuseki configuration leaves it commented out i.e. disabled.

I wonder if it would be easy to also check the property definitions from within the current vocabulary itself, as is the current behavior?

[tfrancart]

Thanks for reviewing. I have modified the method so that it first queries the current vocabulary for property labels/superprops, before defaulting to the default SPARQL endpoint.
Note that for properties with no label at all anywhere, this will make another additionnal unnecessary query.

[osma]

Thanks, but that's probably not the best way to do it. The property definitions (including label and rdfs:subPropertyOf) are already included in the CONSTRUCT query result for the current concept, so there's no need to perform yet another SPARQL query. See the original code around line 355 in Concept.php that you deleted already in the first version of this PR. There is no need to perform another SPARQL query to get that information if it's already in the graph we have. Only if it's not there does it make sense to query the default SPARQL endpoint.

[tfrancart]

OK done.

[henriyli]

Thank you!