iovka / shex-java

Validation of Shape Expression Schemas
GNU Lesser General Public License v3.0
10 stars 6 forks source link

Commons RDF? #2

Closed ajs6f closed 6 years ago

ajs6f commented 6 years ago

This is a very neat project, and I am thinking about using it as the basis of a validation module for Apache Jena. I see that you are recommending using RDF4J but you do have support for Jena. Would you be interested in a PR that puts shex-java over Apache Commons RDF (which supports Jena, RDF4J, and several other implementations?

jdusart commented 6 years ago

Thanks. Yes, there is a small support for Jena. You have the class fr.inria.lille.shexjava.graph.JenaGraph that allow to use Jena Model. This will convert your Jena IRI, Literal and BNode when asked to RDF4J IRI, Literal and BNode that are used in the implementation. Yes, I will consider using Commons RDF for the next release.

ajs6f commented 6 years ago

Great! I've started the work here.

Unsurprisingly, it's kind of a large changeset, since a lot of code must be touched to use new types. Currently I have:

Tests run: 4280, Failures: 18, Errors: 3, Skipped: 0

although the number of tests-in-error, for some strange reason, seems to vary occasionally. I don't pretend to understand the test framework very well, so that remains to be investigated. All of the failing tests are in TestShExCParserShExJSerializer.

WIP! :) I can open a PR at any point if we want to use Github's commenting and discussion tools.

There are a few design issues:

Please let me know if anything sounds or looks horrible or like I'm going completely wrong or off. I don't pretend for a moment to any deep knowledge of ShEx, so all comments welcome!

ajs6f commented 6 years ago

One other note-- I'm trying to do a very literal "translation" to start with. In other words, although the Commons RDF API is different not just in name but also in functionality from RDF4J, I'm trying not to use any more of the differences than are absolutely necessary. If and when this work seems acceptable, I can continue on and make use of some of the differences, mostly to shorten the code here and there.

jdusart commented 6 years ago

Great work but before going any further, this week I look at it and have started a new branch commons which is the passage to dev to commons. This pass most of the tests. Also be careful when you launch the tests, because there are some problems or disagreement with some tests in it (we are currently looking to solve those problems), the implementation is not returning a test failure when a test fails to not create problems with maven build. You need to look at the trace of the maven test run to know the tests that it's failing.

For the implementation:

ajs6f commented 6 years ago

Yes, I saw that test failures don't fail the test, so to speak. So... should I abandon my branch? I have no desire to waste time redoing work.

ajs6f commented 6 years ago

As to the two branches, it sounds like we made the same decisions, which is generally a good sign!

jdusart commented 6 years ago

I also did in my branch some small code reorganization, so I prefer that we keep mine.

ajs6f commented 6 years ago

I will close my PR. As I wrote, my concern is to reuse this module with Jena, so please close this issue when you merge your branch so that I will know that I can move forward. Thanks!

ajs6f commented 6 years ago

Ah, just realized that I never issued the PR, so nothing to close.

jdusart commented 6 years ago

I have just released on maven the version 1.1a with better support for commonsRDF.

ajs6f commented 6 years ago

Great, thanks! I will try to build up a jena-validator module around it.