Adding skolemize to the SparqlHandler

[danielbeeke]

See #262

[jeswr]

Thanks @danielbeeke, the implementation looks good at a glance! Would you be able to add some tests that cover this new functionality to prevent regressions in the future?

[danielbeeke]

Ill will try to do this in the following weeks, I have a quite busy month.

[danielbeeke]

I am sorry for the long time this has been open. I added the regression test. @jeswr is the test okay this way?

[jeswr]

The test is looks good!

Having done a fair amount with blank nodes on Comunica since this PR I don't think that this behavior is going to be sufficient in most cases. In particular I think that we should first be checking if the blank node is already scoped (e.g. like https://github.com/constraintAutomaton/comunica/blob/9dc78b2ca25378bc0a538f0c30346b655e9def76/engines/query-sparql/test/QuerySparql-test.ts#L118-L122) and if it is then we should use that scoping rather than applying the scope from LDflex.

Do you have the time to take this on @danielbeeke ? I totally understand if not.

Your work/contributions so far are greatly appreciated.

[danielbeeke]

Have you seen?
https://github.com/LDflex/LDflex/blob/master/src/SparqlHandler.js#L217

If the blank node was already skolemized it will keep that label.

If have checked your reference. And:
https://github.com/comunica/comunica/blob/master/packages/data-factory/lib/BlankNodeScoped.ts

With a BlankNodeScoped Comunica could use that information when writing back? I do not yet fully get it, when is that useful? When copying data from one to another source? Is there more information that I could read about it?

I can see if I understand the issue you mention, and try to solve it.

[jeswr]

Have you seen? https://github.com/LDflex/LDflex/blob/master/src/SparqlHandler.js#L217

Ah my bad - was still waking up when I did this review.

Thanks for your work! I'm happy with this PR, and will create open an issue to add better test coverage for BlankNodeScoped nodes which are caught by that line.

[jeswr]

🎉 This PR is included in version 2.15.2 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀