search

RDFLib / pySHACL

A Python validator for SHACL
Apache License 2.0
241 stars 63 forks source link

Remove -Likes #229

Open ajnelson-nist opened 2 months ago

ajnelson-nist commented 2 months ago

While trying to understand some of the code base's typing a bit better, I checked the definition of GraphLike and found it was not fulfilling the same role as "-like" parameters I had seen in RDFLib. This PR removes replaces types specialized to pySHACL with classes used more broadly in RDFLib. If it turns out there was another purpose these -Likes were serving, I don't mind this PR being NACKed and closed, though I'd be curious to hear about the use case.

This PR builds on PR 228. The first patch in the PR documents specific rationales on removing GraphLike and ConjunctiveLike. Some parameter type revisions around ConjunctiveGraph vs. Dataset are intentionally left out of the first patch, and I leave it to the maintainers whether they should be added to this PR.

ashleysommer commented 2 months ago

Hi @ajnelson-nist

Thanks for this. The removal of these GraphLike types was something I had been planning to get around to.

For some context, these were introduced at a time before RDFLib had any typing, this was my attempt to make sense of the Graph class hierarchy in rdflib, and make it compatible with the PySHACL typing expectations. The situation since then in RDFLib has improved, so we can probably use their types now.

Note, the solution here may not be as simple as it seems, because RDFLib is planning (has been planning for at least the last 3 years) to remove the ConjunctiveGraph class, and have Dataset inherit directly from Graph. I believe if they keep ConjunctiveGraph for compatibility reasons, it will be a subclass of Dataset. That was one reason for the ConjunctiveLike union I had included here, and your solution of simply replacing it with rdflib.ConjunctiveGraph will not suffice.

This may require further thought.

nicholascar commented 2 weeks ago

ConjunctiveGraph is on the chopping block for RDFLib 8.0.0 likely due in later 2024.

Yes, I think Dataset will be a subclass of Graph but I do want to entirely remove ConjunctiveGraph, not keep it at all, just to reduce toolkit terminology and complexity.