#491: Added node expression test cases and clarified spec in two places #719
Conversation
HolgerKnublauch
requested review from
recalcitrantsupplant,
robert-david and
simonstey
ajnelson-nist
left a comment
ajnelson-nist
left a comment
There was a problem hiding this comment.
I think there are conversation threads in exists.ttl, instancesOf.ttl/nodesMatching.ttl, and var.ttl that should be resolved before this is merged.
For some logistics reasons, please consider addressing those threads as my "Request changes", but I won't push "Request changes" on GitHub in case I end up unavailable to unblock in a timely manner.
Co-authored-by: Alex Nelson <[email protected]>
|
Thanks Alex, for your review and comments. I hope I have addressed them. I went ahead and marked the threads as resolved but please feel free to reopen if you disagree or have further questions. |
Also restored alphabetic order in shnex, causing a larger diff (but only the stuff about shnex:orderBy is new)
robert-david
left a comment
robert-david
left a comment
There was a problem hiding this comment.
I approve to not block it further, but please look at my comments. Thanks
| <code>nodes</code> be the <a>value</a> of <code>shnex:nodes</code> and | ||
| <code>desc</code> be the <a>value</a> of <code>shnex:desc</code> in the <a>order by expression</a>. | ||
| Let <code>N</code> be the <a>output nodes</a> of <code>evalExpr(nodes, focusGraph, focusNode, scope)</code>. | ||
| Let <code>c(n)</code> be the first output node of <code>evalExpr(orderBy, focusGraph, n, scope)</code> |
There was a problem hiding this comment.
I don't understand what c(n) returns. Why is it a single (first) node? I guess this should be an ordering function or returning and index, like described in '...sorted by c(n)...'. Also, shouldn't the value of shnex:desc be included in the evalExpr?
There was a problem hiding this comment.
Basically c(n) returns one value for each node from the input list. Then these (derived) values are sorted. For example, assume we want to sort IRIs by strlen
sh:values [
shnex:nodes [ <urn:Medium> <urn:Short> <urn:VeryLong> ] ;
shnex:orderBy [ sparql:strlen ( shnex:var "focusNode" ) ] ;
]
then c(n) produces a list like [ 10 9 12 ] where the first item represent urn:Medium etc.
Can anyone suggest how to make this clearer in the definition?
Desc cannot be included in evalExpr because that has a fixed set of parameters. However, desc is used as a post-processing step here, on the resulting list.
Co-authored-by: Robert David <[email protected]>
| produced by each <a>node expression</a> <code>NE</code> in <code>members</code>, using <code>evalExpr(NE, focusGraph, focusNode, scope)</code>. | ||
| Nodes must be equal using <a>term equality</a>, i.e. <code>"01"^^xsd:integer</code> is distinct from <code>"1"^^xsd:integer</code>. | ||
| Nodes must be equal using <a>term equality</a>, e.g., <code>"01"^^xsd:integer</code> is distinct from <code>"1"^^xsd:integer</code>. | ||
| The intersection does not include duplicates and the order is undefined. |
There was a problem hiding this comment.
| The intersection does not include duplicates and the order is undefined. | |
| The intersection does not include duplicates, and the order is undefined. |
There was a problem hiding this comment.
@TallTed the PR was already merged and closed. I suggest we do a final read-through of these documents before they are considered frozen and moved to the next stage, to fix commas etc. Creating another PR just for one line is just not scalable.
Closes #491
This is meant to be a START only and there certainly can and should be more tests in the future.