Fixes inconsistency in the definition versus the use of the 'Filter' algebra operator #255
Conversation
| We define:</p> | ||
| <p><a href="#defn_algFilter" class="algFct">Filter</a>(<var>expr</var>, <var>Ω</var>, |D|, |G|) = { <var>μ</var> in <var>Ω</var> | <var>expr</var>(<var>μ</var>) is an expression that has an | ||
| effective boolean value of true }</p> | ||
| <p><a href="#defn_Multiplicity">multiplicity</a>( <var>μ</var> | <a href="#defn_algFilter" class="algFct">Filter</a>(<var>expr</var>, <var>Ω</var>) ) | ||
| <div class="issue" data-number="254"> | ||
| It is not clear what <var>expr</var>(<var>μ</var>) is, and it is not apparent in the formula that the expression |expr| is meant to be evaluated not only with respect to <var>μ</var> but also with respect to |D| with active graph |G|.</div> | ||
| <p><a href="#defn_Multiplicity">multiplicity</a>( <var>μ</var> | <a href="#defn_algFilter" class="algFct">Filter</a>(<var>expr</var>, <var>Ω</var>, |D|, |G|) ) | ||
| = <a href="#defn_Multiplicity">multiplicity</a>( <var>μ</var> | <var>Ω</var> )</p> |
There was a problem hiding this comment.
It appears likely that these <p> should instead be a list (I think unordered, but I could be wrong) beneath the We define: which I suggest should be changed to We define the following:
There was a problem hiding this comment.
Any other instances of
We define:</p>and subsequent<p>should get similar handling.
I agree with the We define the following:.
Your other suggestion (to present each block of formulas as a list) is a matter of taste, I guess.
In any case, these changes are not related to the purpose of this PR and, thus, I would keep them for a separate PR (to be created after this one is merged). I created an issue to remember this TODO: #258
There was a problem hiding this comment.
I'm not convinced by the " the following" which to me suggests "one or more".
It's not separate items - it's a unit (singular).
There was a problem hiding this comment.
Something usable with "one or more" is usable with "one". Part of the reason for We define the following: is consistency with similar sections that follow this one. Part of the reason is that folks love having the colon before a bullet, but the colon is only correct when it goes where a period could go, i.e., at the end of a sentence. "We define:" is incorrect, because it's a fragment, not a full sentence. "We define the following:" is a full sentence. ("We define the following [collective noun which I cannot now summon to mind]" would be even better, if someone else has a suitable collective noun that can be used in all these similar sections.)
| <a href="#expressions">expression</a>. We define:</p> | ||
| <p><a href="#defn_algDiff" class="algFct">Diff</a>(<var>Ω<sub>1</sub></var>, <var>Ω<sub>2</sub></var>, <var>expr</var>) = { <var>μ</var> | <var>μ</var> in <var>Ω<sub>1</sub></var> such that ∀ <var>μ'</var> in | ||
| <var>Ω<sub>2</sub></var>, either <var>μ</var> and <var>μ'</var> are not <a href="#defn_algCompatibleMapping">compatible</a> or <var>μ</var> and <var>μ'</var> are <a href="#defn_algCompatibleMapping">compatible</a> and | ||
| <var>expr</var>(merge(<var>μ</var>, <var>μ'</var>)) does not have an effective boolean value of true }</p> | ||
| <div class="issue" data-number="254"> | ||
| It is not clear what <var>expr</var>(<var>μ</var>) is, and it is not apparent in the formula that the expression |expr| is meant to be evaluated not only with respect to <var>μ</var> but also with respect to |D| with active graph |G|.</div> | ||
| <p><a href="#defn_Multiplicity">multiplicity</a>( <var>μ</var> | <a href="#defn_algDiff" class="algFct">Diff</a>(<var>Ω<sub>1</sub></var>, <var>Ω<sub>2</sub></var>, <var>expr</var>) ) = | ||
| <a href="#defn_Multiplicity">multiplicity</a>( <var>μ</var> | <var>Ω<sub>1</sub></var> )</p> |
There was a problem hiding this comment.
As with above We define: and subsequent <p>.
| <a href="#expressions">expression</a>. We define:</p> | ||
| <p><a href="#defn_algLeftJoin" class="algFct">LeftJoin</a>(<var>Ω<sub>1</sub></var>, <var>Ω<sub>2</sub></var>, <var>expr</var>) = <a href="#defn_algFilter" class="algFct">Filter</a>(<var>expr</var>, <a href="#defn_algJoin" class="algFct">Join</a>(<var>Ω<sub>1</sub></var>, | ||
| <var>Ω<sub>2</sub></var>)) ∪ <a href="#defn_algDiff" class="algFct">Diff</a>(<var>Ω<sub>1</sub></var>, <var>Ω<sub>2</sub></var>, <var>expr</var>)</p> | ||
| <p><a href="#defn_Multiplicity">multiplicity</a>( <var>μ</var> | <a href="#defn_algLeftJoin" class="algFct">LeftJoin</a>(<var>Ω<sub>1</sub></var>, <var>Ω<sub>2</sub></var>, <var>expr</var>) ) = |
There was a problem hiding this comment.
As with above, and extending down to line 9963.
There was a problem hiding this comment.
Any other instances of We define:</p> and subsequent <p> should get similar handling.
|
As @TallTed's proposed edits are captured in a separate issue now (#258) and @rubensworks is still on vacation I guess, I am going to merge this PR now, based on the approvals of the other three co-editors. |
The main change in this PR, which addresses #212, is to extend the signature of the Filter algebra operator from Filter(expr, Ω) to Filter(expr, Ω, D, G), where D is a dataset and G is the active graph.
Additionally, the PR adds notes about issue #254.
Preview | Diff