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

Add processor state, term constructor, and triple constructors. Change reifiedTriple to reifingTriple. #62

Merged
merged 22 commits into from
Aug 29, 2024

Conversation

gkellogg
Copy link
Member

@gkellogg gkellogg commented Aug 11, 2024

... for annotationBlock, tripleTerm, and reifier.

Also, extract the annotationBlock production from the annotation production.


Preview | Diff

…annotationBlock`, `tripleTerm`, and `reifier`.

* Extract the `annotationBlock` production from the `annotation` production.
@gkellogg gkellogg added the spec:editorial Minor issue or proposed change in the specification (markup, typo, informative text) label Aug 11, 2024
@gkellogg gkellogg requested review from afs and domel August 11, 2024 00:48
Copy link
Member

@TallTed TallTed left a comment

Choose a reason for hiding this comment

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

Mostly small stuff. One big question.

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 Outdated Show resolved Hide resolved
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
@gkellogg gkellogg requested a review from TallTed August 12, 2024 20:32
Copy link
Member

@TallTed TallTed left a comment

Choose a reason for hiding this comment

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

Source lines 1764-1768 may still need work — I'm not sure what "set from" means, compared to "set to" and "taken from". This part of the doc may become clearer with walk-through examples, with real values — and may need that help for readers other than me, too.

@gkellogg
Copy link
Member Author

Source lines 1764-1768 may still need work — I'm not sure what "set from" means, compared to "set to" and "taken from". This part of the doc may become clearer with walk-through examples, with real values — and may need that help for readers other than me, too.

I'd like to get some input from @afs before making more changes, as I believe the original style for processor events may be his invention. We have not previously given examples of how to parse documents, and I don't think it makes sense to do so now. My guess is that most parsers tend to do their own thing and not strictly follow these rules, but they're needed to express the intention for how to turn Turtle into the Abstract Syntax.

spec/index.html Outdated Show resolved Hide resolved
@gkellogg gkellogg changed the title Add processor state, term constructor, and triple constructors Add processor state, term constructor, and triple constructors. Change reifiedTriple to reifingTriple. Aug 14, 2024
@TallTed
Copy link
Member

TallTed commented Aug 14, 2024

Rather than 17 (or more) suggestions, one comment:

reifing should be reifying, in all cases, matching the case of the existing occurence.

@gkellogg
Copy link
Member Author

Rather than 17 (or more) suggestions, one comment:

reifing should be reifying, in all cases, matching the case of the existing occurence.

Fixed! Brain fart in search/replace.

spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved
@hartig
Copy link
Contributor

hartig commented Aug 15, 2024

I am skeptical about the change of reifiedTriple to reifyingTriple.

I understand that expressions accepted by this production are meant to be parsed into a reifying triple (using the term "reifying triple" here as per the current definition in PR w3c/rdf-concepts#98). But conceptually, at least for me, such an expression is something else than a reifying triple, and calling such an expression a "reifying triple" may be a source for confusion.

For instance, the expression

<< ex:bob  foaf:knows  ex:alice >>

matches the production. If we were to refer to this expression as a "reifying triple", I guess many users would assume that the triple (ex:bob, foaf:knows, ex:alice) is the one that we refer to as being "reifying", but it is not. Instead, the reifying triple is the one that this expression expands to; namely:

_:b rdf:reifies <<( ex:bob  foaf:knows  ex:alice )>> .

As a separate but related comment, I think that replacing the term "reified triple" by "reifying triple" as currently done in this PR -- namely, in the first two sentences of Section 2.10 -- is wrong. I created two separate review comments for these two sentences (comment 1 and comment 2).

@gkellogg
Copy link
Member Author

As a separate but related comment, I think that replacing the term "reified triple" by "reifying triple" as currently done in this PR -- namely, in the first two sentences of Section 2.10 -- is wrong. I created two separate review comments for these two sentences (comment 1 and comment 2).

I disagree, but I'm open to alternatives. We need the concept reifying triple, and the reifyingTriple production does result in the spelled out form, so seems appropriate to me.

@TallTed
Copy link
Member

TallTed commented Aug 16, 2024

I am taking this thread out of the context of any single suggestion, because it will get lost there (as have previous conversations).

We need the concepts of both a Reifying Triple and a Reified Triple.

These are distinct.

We have been talking about both for quite some time, and still we have gone back-and-forth with identifying the same thing as being an instance of one of these concepts and then the other.

A Reifying Triple is a triple that has a predicate of rdf:reifies.

A Reified Triple is the object of a triple that has a predicate of rdf:reifies.

<< ex:bob foaf:knows ex:alice >> might be read has having left out the optional ~ :reifier (and what seems to me to be an optional but necessarily implied .) following ex:alice, making this the complete expression:

<< ex:bob foaf:knows ex:alice ~ [] . >>

That's sugar for the actual reifying triple, which is

[] rdf:reifies <<( ex:bob foaf:knows ex:alice . )>> .

That reifies the now-reified triple,

ex:bob foaf:knows ex:alice .

I hope that's all clear....

@gkellogg
Copy link
Member Author

A Reified Triple is the object of a triple that has a predicate of rdf:reifies.

We call that a Triple Term.

<< ex:bob foaf:knows ex:alice >> might be read has having left out the optional ~ :reifier (and what seems to me to be an optional but necessarily implied .) following ex:alice, making this the complete expression:

<< ex:bob foaf:knows ex:alice ~ [] . >>

The following are equivalent:

  • << ex:bob foaf:knows ex:lice >>
  • << ex:bob foaf:knows ex:lice ~ >>
  • << ex:bob foaf:knows ex:lice ~ [] >>

They are all the same as [] rdf:reifies <<( ex:bob foaf:knows ex:lice )>> .

Note that the reifiedTriple (or whatever we finally settle on) production does not include the . in the grammar.

…r describes how the production relates to the `reifying triple` definition in RDF Concepts.
Copy link
Contributor

@domel domel left a comment

Choose a reason for hiding this comment

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

Now looks good.

Copy link
Contributor

@hartig hartig left a comment

Choose a reason for hiding this comment

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

Just a few minor things.

spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved
Co-authored-by: Olaf Hartig <olaf.hartig@liu.se>
@TallTed
Copy link
Member

TallTed commented Aug 19, 2024

A Reified Triple is the object of a triple that has a predicate of rdf:reifies.

We call that a Triple Term.

In other words, "we call the object of a triple that has a predicate of rdf:reifies a Triple Term" instead of calling it the rather more intuitive "Reified Triple".

At the risk of howls of "bike-shedding", that says to me that rdf:reifies and reifiedTriple should be revisited, in favor of either a different verb or something that's not a verb at all.

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
Finishing the <a href="#grammar-production-annotation"><code>annotation</code></a> production
yields the RDF triple |TI| <code>rdf:reifies</code> |TT|
yields the RDF triple |curReifier| <code>rdf:reifies</code> |curTripleTerm|,
Copy link
Contributor

Choose a reason for hiding this comment

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

The rdf:reifies triple is generated in AnnotationBlock.

The "annotation" production is annotation ::= (reifier | annotationBlock)* so I don't think there is much to do here especially if the rdf:refies triple is generated at the start of an annotationBlock and all reifier needs to do is set curReifier which gets carried to the next block only (or the reifiedTriple case).

Copy link
Member Author

Choose a reason for hiding this comment

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

As mentioned above, this is needed to emit the reifying triple if |curReifier| was not consumed in an Annotation Block. For example :s :p :o ~ :r.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK - I will reread but I thought a reifing triple was created in both parts for ~:r {| :q :z |}. Maybe Annotation Block only handled the "no explicit reifier case".

And :s :p :o :~r1 ~:r2 .

spec/turtle.bnf Outdated Show resolved Hide resolved
spec/turtle.bnf Show resolved Hide resolved
@gkellogg gkellogg requested a review from afs August 24, 2024 21:30
spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated
@@ -1802,42 +1814,59 @@ <h3>RDF Triples Constructors</h3>
|curPredicate| <var>N</var> .</span>
</p>

<h4 id="tripleTerm" style="padding-bottom:0; margin-bottom:0;"><span>Triple Terms:</span></h4>
<h4 id="tripleTerm" style="padding-bottom:0; margin-bottom:0;"><span>Triple Terms</span></h4>

<p>Beginning the <a href="#grammar-production-tripleTerm"><code>tripleTerm</code></a> production
records the |curSubject| and |curPredicate|.
Copy link
Contributor

Choose a reason for hiding this comment

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

Small errata to RDF 1.1:

  • It does not say that curSubject and curPredicate are stacks, or alternative the parser state is a stack.
  • It says in 7.3 "The curSubject is bound to the subject production." True but it is more than just subject even in RDF 1.1 (subject position, blankNodePropertyList, property lists in the object position and collections). RDF 1.2 adds the tripleTerm, reifiedTriple, annotation and annotationBlock cases.

Copy link
Member Author

Choose a reason for hiding this comment

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

Neither curSubject nor curPredicate (nor the parser state in general) is a stack, they are global state. Triple constructors save and restore their values, so the "stack" is implemented via the activation records of these constructors, such as the Triple Term constructor.

spec/index.html Outdated
Comment on lines 1828 to 1829
Finishing the <a href="#grammar-production-reifier"><code>reifier</code></a> production,
|curReifier| is taken from the <a href="#handle-reifier"><code>reifier</code></a> term constructor.</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete this text. Moved to earlier.

Copy link
Member Author

Choose a reason for hiding this comment

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

The reason it is written the way it is is that Reifier can set the value that might be used in Annotation Block, but we'll only know that if there is an Annotation Block after the Reifier. If it was set, but not used, and we see another Reifier, it can then be emitted for the saved curReifier.

spec/index.html Outdated
<h4 id="reifier" style="padding-bottom:0; margin-bottom:0;"><span>Reifier</span></h4>

<p>Beginning the <a href="#grammar-production-reifier"><code>reifier</code></a> production,
if |curReifier| is set, the production yields the RDF triple |curReifier| <code>rdf:reifies</code> |curTripleTerm|.
Copy link
Contributor

Choose a reason for hiding this comment

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

if |curReifier| is set,

It is not set until the end of the production so it is that previous sate at this point.

The production will set curReifier - there 's no "if". Do this now and not at the end, then yield the reifies triple using curReifier.

Suggested change
if |curReifier| is set, the production yields the RDF triple |curReifier| <code>rdf:reifies</code> |curTripleTerm|.
|curReifier| is taken from the <a href="#handle-reifier"><code>reifier</code></a> term constructor.
For the |tripleTerm| from the last RDF triple yielded then yield the the RDF triple |curReifier| <code>rdf:reifies</code> |curTripleTerm|.

There is no need to have a stack for curReifier. Even in annotations, only the last value is needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

The text is intended to capture the events generated by :s :p :o ~:r1 ~:r2. When seeing ~:r1 there is no previously established curReifier so nothing is emitted, then curReifier is set from the reifier term constructor. The next time when it sees ~:r2, curReifier would be :r1 so :ri rdf:reifies <<(...)>> is emitted and then curReifier is set to :r2.

There is no stack for curReifier, there is just if it was previously set or not (Note that there is no text that says to save the value of curReifier anywhere). It is cleared when entering Annotations, set in either ReifierorAnnotation Block, and consumed in Reifier, Annotation BlockorAnnotations`.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm beginning to see what you want to do here but then some text does seem right.

Finishing the annotation production yields the RDF triple curReifier rdf:reifies curTripleTerm, resets the value of curReifier

needs a condition that curReifier is set. In :s :p :o ~:r {| :x :y |} it is not (it's been used). That's where I see identical triples being emitted once for the annotationBlock and one at the and of annotations.

There needs to be more recording and restoring:

  :s :p :o ~:r1 {| :x1 :y2 ~:r2 {| :xx1 :yy2 |} ;
                   :x2 :y2
                 |}.

which I think is:

:s :p :o .
:r1 rdf:reifies <<( :s :p :o )>> .
:r1 :x1 :y2 .
:r2 rdf:reifies <<( :r1 :x1 :y2 )>> .
:r2 :xx1 :yy2 .
:r1 :x2 :y2 .

note :r1 :x2 :y2.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's what my parser produces.

spec/index.html Outdated

<h4 id="reifier" style="padding-bottom:0; margin-bottom:0;"><span>Reifiers:</span></h4>
<h4 id="reifiedTriple" style="padding-bottom:0; margin-bottom:0;"><span>Reified Triple</span></h4>

<p>Beginning the <a href="#grammar-production-reifiedTriple"><code>reifiedTriple</code></a> production
records the |curSubject| and |curPredicate|.
Copy link
Contributor

Choose a reason for hiding this comment

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

In the parser state section, maybe explain "record" means a stack.

<a href="#grammar-production-reifier"><code>reifier</code></a> of the
<a href="#handle-reifiedTriple"><code>reifiedTriple</code></a> term constructor
if present,
or a fresh RDF blank node, if not matched.
A new <a href="#grammar-production-tripleTerm"><code>tripleTerm</code></a> instance |TT|
is composed from
the <a href="#grammar-production-subject"><code>subject</code></a>,
Copy link
Contributor

Choose a reason for hiding this comment

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

It may not be the subject production. It can be reifiedTriple. They both set curSubject but that is used used later as the reifier carried over.

spec/index.html Outdated
A new <a href="#grammar-production-tripleTerm"><code>tripleTerm</code></a> instance |TT|
is composed from
the <a href="#grammar-production-subject"><code>subject</code></a>,
<a href="#grammar-production-predicate"><code>predicate</code></a>, and
<a href="#grammar-production-object"><code>object</code></a> productions.
Finishing the <a href="#grammar-production-reifier"><code>reifier</code></a> production
Finishing the <a href="#grammar-production-reifiedTriple"><code>reifiedTriple</code></a> production
yields the RDF triple |curSubject| <code>rdf:reifies</code> |TT|
Copy link
Contributor

@afs afs Aug 26, 2024

Choose a reason for hiding this comment

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

curSubject may have been modified by the subject/reifiedTriple in the subject position inside << >>.
It might be batter use curReifier and make it a stack (for annotation and asserted triples it does not need to be a stack as in comment above.

spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved
gkellogg and others added 4 commits August 26, 2024 09:15
Co-authored-by: Andy Seaborne <andy@apache.org>
* Try to make all variables highlight across sections.Triple Contructor, don't emit the reifying triple unless |curReifier| is set.
spec/index.html Outdated
@@ -1802,42 +1814,59 @@ <h3>RDF Triples Constructors</h3>
|curPredicate| <var>N</var> .</span>
</p>

<h4 id="tripleTerm" style="padding-bottom:0; margin-bottom:0;"><span>Triple Terms:</span></h4>
<h4 id="tripleTerm" style="padding-bottom:0; margin-bottom:0;"><span>Triple Terms</span></h4>
Copy link
Member Author

Choose a reason for hiding this comment

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

I now think that this handler is not needed and can be eliminated.

@gkellogg gkellogg requested a review from afs August 26, 2024 19:36
@gkellogg
Copy link
Member Author

@afs, I re-worked the Triple Constructors partly based on your feedback, but it's possible I missed some of your points.

  • Removed the "Triple Terms" section, which didn't seem to be needed.
  • The Reifier handler now always emits a reifying triple and sets curReifier.
  • The Annotation Block handler sets curReifier if not set, in which case it assigns a blank node and emits the reifying triple.
  • The Reified Triples handler sets curTripleTerm and produces the node based on curReifier.
  • The Annotations handler sets curTripleTerm and stacks curReifier similar to curSubject and curPredicate.

…ationBlock, as it is only used on entry to AnnotationBlock, so restoring has no use.
Copy link
Contributor

@afs afs left a comment

Choose a reason for hiding this comment

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

The best way forwards is to merge this PR. There are significant updates so making those visible to early implementers will enable feedback.

@pchampin
Copy link
Contributor

This was discussed during the rdf-star meeting on 2024-08-29:
https://www.w3.org/2024/08/29-rdf-star-minutes.html#t04

@gkellogg gkellogg merged commit bc0650c into main Aug 29, 2024
2 checks passed
@gkellogg gkellogg deleted the update-annotation-processor-instructions branch August 29, 2024 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec:editorial Minor issue or proposed change in the specification (markup, typo, informative text)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants