refactor: let rdflib decide where collection syntax is safe - #4
Merged
Merged
Conversation
deterministic_turtle carried a TurtleSerializer subclass that re-decided
where ``( … )`` collection syntax may be used, rejecting any chain with a
cell more than one statement points into. That decision belongs to rdflib,
and it now makes it: the same condition is implemented in both of rdflib's
Turtle serializers upstream.
Two code paths claiming the same thing is one too many, so the subclass goes
and the first render uses rdflib's serializer directly. What stays is the
part that is this library's own: rendering, checking the text round-trips,
and falling back to explicit rdf:first/rdf:rest statements when it does not.
That check is what makes the guarantee hold, and it holds whichever way
rdflib decides - so this needs no rdflib floor bump, and a future rdflib
that inlines more aggressively cannot silently break the canonical form.
Verified inert, not assumed:
- the deleted subclass and rdflib's serializer emit BYTE-IDENTICAL text for
all four ASAM OpenX artifacts it is used on (opendrive/openscenario, OWL
and SHACL; 8,089 / 5,476 / 15,203 / 11,040 triples). Same text means the
same round-trip verdict, the same fallback branch and the same output, so
no committed artifact changes and nothing needs regenerating.
- 188 tests pass, including the six sharing arrangements in SHARING_CASES,
which exercise the fallback rather than the subclass now.
- a private list still gets ``( … )``; a shared list still falls back; a
graph mixing both keeps ``( … )``. Readability is unchanged.
Removes the now-unused RDF import.
Signed-off-by: jdsika <carlo.van-driesten@vdl.digital>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
deterministic_turtlecarried aTurtleSerializersubclass that re-decided where( … )collection syntax may be used, rejecting any chain with a cell more than one statement points
into. That decision belongs to rdflib — and it now makes it, with the same condition, in both of
its Turtle serializers (RDFLib/rdflib#3504).
Two code paths claiming the same thing is one too many. The subclass goes; the first render uses
rdflib's serializer directly.
What stays
The part that is this library's own job: render, check the text round-trips, and fall back to
explicit
rdf:first/rdf:reststatements when it does not. That check is what makes theguarantee hold, and it holds whichever way rdflib decides.
Two consequences worth naming:
so a floor could not name it anyway. Because correctness comes from the check rather than from
the serializer's choice, the library is correct on any
rdflib>=6.0and picks up nicer outputautomatically when a fixed rdflib ships — no second change needed.
Verified inert, not assumed
artifacts this is used on —
opendriveandopenscenario, OWL and SHACL (8,089 / 5,476 /15,203 / 11,040 triples). Identical text means the same round-trip verdict, the same fallback
branch, and the same final bytes: no committed artifact changes, nothing needs regenerating.
SHARING_CASESnow exercise the fallback rather thanthe subclass, so the sharing cases are still covered end to end.
( … ), a shared list still falls back, and agraph mixing both keeps
( … ).Note on those artifacts
On the real ASAM graphs the deleted subclass was already a no-op — rdflib's inline form does not
round-trip them (openscenario gains 74 triples, opendrive 152, in the
sh:inenumeration lists),and the subclass produced exactly the same broken text, so the collection-free fallback is what
has been producing those files all along. Their correctness rests on the round-trip check, which
is precisely the part this PR keeps.
Worth a separate look, and deliberately not bundled here: those graphs round-trip only via the
fallback even though every chain containing a shared cell is rejected, which suggests a second
inlining path that neither condition covers. If that reproduces standalone it is a further rdflib
report, not something to work around here.
Not bundled
The module and test docstrings still carry provenance references (
linkml#3295,issue #1) thatsit oddly with "docs state the current design". Left alone — unrelated to this change.