-
Notifications
You must be signed in to change notification settings - Fork 13
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
fix: problem with assumption that "quoted triples" are quads #35
base: master
Are you sure you want to change the base?
Conversation
|
rdf-js-tests.ts
Outdated
const term: Term = <any> {}; | ||
switch (term.termType) { | ||
case 'Quad': | ||
const itIsQuad: BaseQuad = term; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sadly, I realized that this is in fact a breaking change because we now have two term types "Quad"
I have not found a good way to address this so far
As repeatedly argumented in the same issue #34, quoted triples are quads, just like all other triples. The fact that their graph information is irrelevant for some operations does not mean it should be left out. |
I agree with @woutermont here. I also want to echo #34 (comment) in that it is ok for these interfaces to be more general than what is allowed by the spec. It is something that is done already in RDFJS with one example being The introduction of a new export type Quad_Subject = NamedNode | BlankNode | Triple | Variable;
export type Quad_Predicate = NamedNode | Variable;
export type Quad_Object = NamedNode | Literal | BlankNode | Triple | Variable;
export type Quad_Graph = DefaultGraph | NamedNode | BlankNode | Variable;
export interface Triple extends Quad {
graph: DefaultGraph; // or null if we did go with option 1
} But as this is breaking I suggest that we do not make such a change at present; especially since the RDF-star WG has only just started. |
Well, that's the thing. We don't: const fact = $rdf.quad(Ruben, says, $rdf.quad(elephants, color, "yellow"))
console.log(isQuoted(fact))
function isQuoted(quad: Quad): boolean {
return ??
} Inside the function
Here, I opt for 3, because it is closes to the terminology of "quoted triple" which is not a quad. |
We can keep on repeating this, but a triple/quad is neither quoted nor asserted unless in a specific moment of usage. So it is perfectly normal not to "know" of a quad which "kind" it is: there are no kinds, only usages. |
That code snippet I would equate to trying to do const term = $rdf.namedNode('http://example.org#Jesse')
console.log(isSubject(fact))
function isSubject(term: Term): boolean {
return ??
} whether or not it occurs in the subject of any Triple/ In the same way I could have const fact1 = $rdf.quad(Jesse, a, Person)
const fact2 = $rdf.quad(Ruben, says, fact1)
store.addQuads([
fact1,
fact2
]) Here EDIT Fixed to use fact1 as a term in fact2 |
You subject is a good analogy. In the default const quad1 = $rdf.quad(knows, a, Property)
const quad2 = $rdf.quad(Jesse, quad1.subject, Wouter) This may be correct, but the types My proposal addresses a similar problem, which may cause unexpected results when you take a quoted triple and try to assert it in your dataset. If you take it at face value, it may end up not in the graph you expect. This will inevitably lead to bugs. |
@tpluscode - I just realised I made a mistake in #35 (comment) and have now fixed it. The entire point I was trying to make was that fact1 can be both quoted and asserted based on the context it is used. Note that what I have above is now equivalent to Jesse a Person
Ruben says <<Jesse a Person>> |
Indeed, I think I noticed that but my brain adjusted to interpret as you intended. Note again that this is not exactly equivalent to my snippets. You create 2 quads and assert them. No issues there. The problem when you would write instead: const fact1 = $rdf.quad(Jesse, a, Person)
-const fact2 = $rdf.quad(Ruben, says, fact1)
-const fact2 = $rdf.quad(Ruben, says, fact1, graph)
store.addQuads([
- fact1,
+ fact2.object,
fact2
]) Here's a practical example, if that might help. Code which will apply a set of changes to a graph (triples to add and remove). const $rdf = require('[email protected]')
const N3 = require('n3')
const ex = require('@rdfjs/[email protected]')('http://example.com/')
const getStream = require('[email protected]')
const dataBefore = `
@prefix : <http://example.com/>.
graph :G {
:elephant :color "Yellow"
}`
const changes = `
@prefix : <http://example.com/>.
graph :G {
[
:add << :elephant :color "Grey" >> ;
:remove << :elephant :color "Yellow" >> ;
] .
}`
const parser = new N3.Parser()
const dataset = $rdf.dataset(parser.parse(dataBefore))
for(const mod of parser.parse(changes)) {
if (mod.predicate.equals(ex.add)) {
dataset.add(mod.object)
} else {
dataset.remove(mod.object)
}
}
await getStream(dataset.toStream().pipe(new N3.StreamWriter({
prefixes: { '': 'http://example.com/' }
}))) Run it here: https://runkit.com/embed/v50w7w5v253b and you will get this output @prefix : <http://example.com/>.
:G {
:elephant :color "Yellow"
}
:elephant :color "Grey". The result is not what you want, and likely not what you'd expect, with the new triple added to default graph and the removed triple not actually removed. This PR adds types to prevent such mistake and I would strongly consider actually failing in runtime when trying to assert a quoted triple directly like that. Here's how the code would be modified to match expectation for(const mod of parser.parse(changes)) {
+ const { subject, predicate, object } = mod.object
+ const modifiedQuad = $rdf.quad(subject, predicate, object, ex.G)
if (mod.predicate.equals(ex.add)) {
- dataset.add(mod.object)
+ dataset.add(modifiedQuad)
} else {
- dataset.remove(mod.object)
+ dataset.remove(modifiedQuad)
}
} |
Except for the fact that with the proposed changes #35 (comment) would have a type error because
I now see your point; I'm just not sure whether this is likely to really be a source of error in practice; and I fear it would just result in more time spent dealing with typing headaches + adressing breaking changes across existing libs, than time solved debugging. That said I will think on this over the weekend. |
It shouldn't, because |
@tpluscode I think that elaborate example pictures very clearly what you have been trying to say, but I still do not agree. The result is exactly what I would expect, and what I would want when writing those lines. You start from this dataset:
You loop through the changes:
Notice how the graph name :elephant :color "Yellow" :G .
+ :elephant :color "Grey" :default .
- :elephant :color "Yellow" :default . If we wanted to remove the initial triple, we would need a more expressive language, allowing us to write:
Exactly because of this need for more expressiveness, I expect such a language to rise (probably as an RDF-Star extension), which is why I argue that we should untill then just use the default graph for quoted triples, as this would be in line with the expected behavior of such a language. Adding a more complex type system (e.g. with Triples) now would only have to be reverted when we reach that point in time. |
This is exactly where we differ, and IMO you deviate from the RDF-star spec. The quoted triple is placed in graph Consider a hypothetical REST API scenario, in which these changes are sent as a turtle document, such as the difference triples would not necessarily be in the same graph PATCH /resource/G
Content-Type: text/turtle
_:blank :add << :elephant :color "Grey" >> .
_:blank :remove << :elephant :color "Yellow" >> . It is unspecified how the body is parsed, likely as a set of triple in default graph
Maybe, but that is not the language we have.
I would not be so sure. It is designed that way for good reason and if you read how the triple vs quad is interpreted at the core of RDF, you should expect the only triples will every be "quoted".
Not necessarily. I expect a quoted triple would still have that ambiguous semantics due to the syntactic impossibility of distinguishing a triple without context from a triple in default graph. Probably another reason why "quoted quads" will never come to pass. |
@tpluscode Like in any other RDF/JS triple vs. quad discussion, I would like to know the problem you can't solve? You build the named graph from the HTTP path and headers, but mapping the quoted triple from the default graph to the target graph is a problem? You can do it with the current interfaces. Not having an explicit mapping step in the patch code could lead to readability problems. |
I think you got it backwards. As outlined in my earlier comments, I do not want to solve problems but avoid them |
Of course I do! But so do you 🙃 The difference is that I explicitly say so in practically every comment. Let's retrace our steps, with regard to @bergos' question about which problem we are trying to solve.
To respond to @tpluscode's criticisms:
That is not the issue. I am not claiming we should write to another language. I am simply making some theoretical, semantic arguments that agree with the practical intuitions of @jeswr and @rubensworks, rather than with yours.
While one can never be 100% sure, it is a fairly safe bet to say that [A] if expressiveness is lacking a language will rise that enables it, or more general [B] if something is useful, someone will build it, and people will use it.
Everything RDF-Star adds is technically possible in RDF itself (heck, even RDF 1.1 is entirely reducable to RDF 1.0 I believe). It's all syntactic sugar, just as my fictional extension of them, so they all share that same "good reason".
If we expect everything to stay the same based on what is explicitly written, we would live in a very poor world. Nowhere in RDF 1.0 did it say "dataset", yet then we got RDF 1.1; but nowhere in that one could I read "quoted triple", and still we got RDF-Star. Now, since RDF-Star does not seem to prohibit extension, I am fairly certain of my expectation for "quoted quads" (even if that will not be their name). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do agree that a Triple
interface should be added but I don't agree with the termType
being QuotedTriple
.
I think a triple
factory method should be added to DataFactory
interface to allow creation of a triple. When the triple is used as the subject or object of a quad, then it is called a quoted-triple but it is still a triple with no graph.
When a triple is added to a Dataset
, it gets added to the default graph. But allowing a graph term to be specified along with the triple would be useful.
To accommodate adding a triple to a Dataset
, the add
method would need to be overloaded to accept a Triple
and an optional graph term.
const triple = $rdf.triple(ex.elephants, ex.color, yellow);
const quad = $rdf.quad(
ex.Simon,
ex.says,
triple,
// Optionally declare a graph term. No graph term defaults to default graph.
);
const dataset = $rdf.dataset([
quad,
quad(
triple.subject,
triple.predicate,
triple.object,
// Optionally declare a graph term. No graph term defaults to default graph.
),
// The following should be allowed too. It will go into the default graph.
triple,
]);
dataset.add(triple); // Goes in default graph.
const graph = $rdf.namedNode('ex:graph');
dataset.add(triple, graph);
Similar changes for the delete
and has
methods.
@@ -233,6 +233,13 @@ export interface Quad extends BaseQuad { | |||
equals(other: Term | null | undefined): boolean; | |||
} | |||
|
|||
export interface BaseTriple extends Omit<BaseQuad, 'graph' | 'termType'> { | |||
termType: 'QuotedTriple'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
termType: 'QuotedTriple'; | |
termType: 'Triple'; |
termType: 'QuotedTriple'; | ||
} | ||
export interface Triple extends Omit<Quad, 'graph' | 'termType'> { | ||
termType: 'QuotedTriple'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
termType: 'QuotedTriple'; | |
termType: 'Triple'; |
case 'Quad': | ||
term2 = <BaseQuad>term; | ||
break; | ||
case 'QuotedTriple': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
case 'QuotedTriple': | |
case 'Triple': |
@@ -113,11 +139,15 @@ function test_datafactory_star() { | |||
); | |||
|
|||
// Decompose the triple | |||
if (quadBobAgeCertainty.subject.termType === 'Quad') { | |||
const quadBobAge2: Quad = quadBobAgeCertainty.subject; | |||
if (quadBobAgeCertainty.subject.termType === 'QuotedTriple') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (quadBobAgeCertainty.subject.termType === 'QuotedTriple') { | |
if (quadBobAgeCertainty.subject.termType === 'Triple') { |
@@ -137,11 +167,15 @@ function test_datafactory_star_basequad() { | |||
); | |||
|
|||
// Decompose the triple | |||
if (quadBobAgeCertainty.subject.termType === 'Quad') { | |||
const quadBobAge2: BaseQuad = quadBobAgeCertainty.subject; | |||
if (quadBobAgeCertainty.subject.termType === 'QuotedTriple') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (quadBobAgeCertainty.subject.termType === 'QuotedTriple') { | |
if (quadBobAgeCertainty.subject.termType === 'Triple') { |
Based on the discussion in #34 I came to the realisation that when RDF* calls "quoted triple" is in fact not a quad. Consider a simple graph:
Currently, this cannot be exactly represented by RDF/JS, because we assume that
:a :name "Alice"
is a quad (spog
), which it is not. It is only a triple. Thus, the code below should fail. At least by type checkingThis PR reintroduces
Triple
type which is simplyQuad
withgraph
removed (and its counterpartBaseTriple
).Note that while this is seemingly a breaking change, in fact it will not affect existing implementation (however they choose to interpret the
graph
property of a quoted triple). Typescript users will find this a breaking change but IMO its is closer to a bug fix of incorrectly interpreting RDF* spec when we first defined it in our type definitions