-
Notifications
You must be signed in to change notification settings - Fork 65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove -Likes #229
base: master
Are you sure you want to change the base?
Remove -Likes #229
Conversation
`GraphLike` and `ConjunctiveLike` appear in several places throughout the pySHACL code, with these definitions in a few files named `pytypes.py`: ```python ConjunctiveLike = Union[ConjunctiveGraph, Dataset] GraphLike = Union[ConjunctiveLike, Graph] ``` In RDFLib 7.0.0 (and according to `git blame`, RDFLib since 2013), the classes referenced in the `Union`s have these subclassing relationships: * `Dataset` is a subclass of `ConjunctiveGraph`. * `ConjunctiveGraph` is a subclass of `Graph`. The RDFLib `Graph` definition designates a few parameters as "-likes" (e.g., `source` in `graph.parse()`), to normalize behaviors around graphs being passed in methods as objects or strings. The Likes in pySHACL do not handle the same use case pattern. This patch replaces the -Likes with their broadest `Union` members, even though a few methods that used `ConjunctiveLike` are described by documentation to prefer `Dataset`. Narrowing this is left for future review. Spelling of `rdflib.Graph` was kept as `rdflib.Graph` for text search purposes. A little cleanup occurs in this patch as well - some type-hinting in comments is mooted by inheritance from function parameters (`validate.py`); `/pyshacl/rdfutil/pytypes.py` is deleted by merit of now being empty; and a `RuntimeError` message has a capitalization typo fixed. Signed-off-by: Alex Nelson <[email protected]>
Thanks for this. The removal of these For some context, these were introduced at a time before RDFLib had any typing, this was my attempt to make sense of the 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 This may require further thought. |
Yes, I think |
|
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
andConjunctiveLike
. Some parameter type revisions aroundConjunctiveGraph
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.