Skip to content
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

LOAD RDF document clarification (description section and definition section) #46

Merged
merged 4 commits into from
Dec 12, 2024

Conversation

afs
Copy link
Contributor

@afs afs commented Nov 26, 2024

This closes #41.

This PR changes the description section for LOAD to cover the case where the RDF Document is a quads format.

There is a note that encountering quads/named graph when expecting triples is an error.

See issue #41 for discussion about that.

This PR is draft at the moment; revisions of the formal LOAD definition section to come.


Preview | Diff

@TallTed
Copy link
Member

TallTed commented Nov 26, 2024

For future similar... It will be helpful for reviewing changes, if the line breaks can be addressed in a distinct commit, if not a distinct PR.

@afs
Copy link
Contributor Author

afs commented Nov 26, 2024

@TallTed The "preview" and "diff" links are useful for that. The WG discussed in the WG meeting of 2024-11-21.

Unfortunately for some reason it is not getting generated (because it was a "draft PR?"). This seems to be an external problem "Timed out after waiting 30000ms" which seems to need an external fix. Hopefully, they will become available.

The changes are restricted to replacing the text within two sections, and are structured as 2 commits (maybe be squashed before merge), one for each section, for ease of review.

spec/index.html Outdated Show resolved Hide resolved
@afs afs marked this pull request as ready for review November 26, 2024 17:48
spec/index.html Outdated
Comment on lines 1182 to 1172
The `LOAD` operation reads an RDF document from a IRI and inserts triples
into the graphs of the dataset in the <a href="#defn_graphStore">Graph Store</a>.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The `LOAD` operation reads an RDF document from a IRI and inserts triples
into the graphs of the dataset in the <a href="#defn_graphStore">Graph Store</a>.
The `LOAD` operation reads an RDF document from a IRI and inserts triples therefrom
into the graphs of the dataset in the <a href="#defn_graphStore">Graph Store</a>.

Copy link
Contributor Author

@afs afs Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"therefrom" is archaic at least in the OED (public access).

and declining

😄

Do you have another suggestion?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The `LOAD` operation reads an RDF document from a IRI and inserts triples
into the graphs of the dataset in the <a href="#defn_graphStore">Graph Store</a>.
The `LOAD` operation reads an RDF document from an IRI and inserts triples from the document
into the graph(s) of a dataset in a <a href="#defn_graphStore">Graph Store</a>.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reworded to be as neutral as possible. (We can't talk about quads.)

The `LOAD` operation reads an RDF document from an IRI and inserts triples
from the document into the <a href="#defn_graphStore">Graph Store</a>.

Section 2 is "The Graph Store" and this runs through the document.

The destination within the graph store is described later.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me. I don't have a "resolved" button, but if you do, please press it.

spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved
spec/index.html Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved
@afs afs changed the title LOAD description section LOAD RDF document clarification (description section and definition section) Nov 27, 2024
Copy link
Member

@rubensworks rubensworks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also update the changelog at the bottom of this document?

@afs afs force-pushed the load-rdf-document branch from e2827f8 to e567e1b Compare November 30, 2024 17:50
@afs afs force-pushed the load-rdf-document branch from 4865fde to a136ddd Compare December 10, 2024 11:03
@afs afs requested review from rubensworks and Tpt December 10, 2024 14:15
Comment on lines +1609 to +1610
triples (from a remote graph or dataset) are added to the Graph Store, either in the
default slot or in a named slot, if specified.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is a "slot"? These seem to be stand-ins for "(named) graph". If "(named) graph" can't be used yet, then this should be changed to --

Suggested change
triples (from a remote graph or dataset) are added to the Graph Store, either in the
default slot or in a named slot, if specified.
triples (from a remote graph or dataset) are added to the Graph Store.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If "(named) graph" can be used, then this should be changed to --

Suggested change
triples (from a remote graph or dataset) are added to the Graph Store, either in the
default slot or in a named slot, if specified.
triples (from a remote graph or dataset) are added to the Graph Store, either in the
default graph or in a named graph, if specified.

Copy link
Contributor Author

@afs afs Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"slots" are described in section 2 and 4.2 Auxiliary Definitions.

The text "either in the default slot or in a named slot, if specified." in LOAD is unchanged from SPARQL 1.1 Update in the formal definition section.

A Graph Store is a mutable container of slots (one unnamed slot, and zero or more slots with name). Each slot holds an immutable graph. Update is "replace graph in slot by by another graph", preserving the container.

A dataset is strictly immutable, just like graphs aren't mutable.

The spec is a bit weak in places:

transforms a Graph Store GS to another Graph Store GS'
written
Op(GS, Args) = GS'

but it is the same Graph Store with different contents.

See the note in 4.2 Auxiliary Definitions that mentions to punning on "Graph Store" and "RDF Dataset".

This could be cleared up (but not this PR!).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad. I should have looked to the pr-preview where it's much clearer why "slots" are being discussed.

@afs afs self-assigned this Dec 11, 2024
@afs
Copy link
Contributor Author

afs commented Dec 12, 2024

See also : w3c/sparql-graph-store-protocol#24.

@afs afs merged commit 3a8757a into main Dec 12, 2024
2 checks passed
@afs afs deleted the load-rdf-document branch December 12, 2024 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow dataset formats to be valid in LOAD with no INTO
5 participants