Support conditions

[abhsarma]

I have created basic condition support using %when% and the branch_assert() I have doubts regarding the function name, and the operator used.

[mjskay]

awesome!

Might be worth creating an issue for "figuring out what the branch API should look like" or something like that. One thing I wonder about is this style:

         mutate( z = branch(values_z,
             "constant" ~ 5,
             "linear" ~ x + 1,
             "sum" ~ (x + y) %when% (values_y == TRUE)
         ))

Versus this style:

         mutate( z = branch(values_z,
             "constant" ~ 5,
             "linear" ~ x + 1,
             "sum" %when% (values_y == TRUE) ~ x + y
         ))

The second one feels more intuitive to me (the option being chosen is what the "when" is about, not what expression it gets turned into). I'm curious if you feel the other is more intuitive?

[mjskay]

Or it might be crazy, but you could override if and do this:

         mutate( z = branch(values_z,
             "constant" ~ 5,
             "linear" ~ x + 1,
             if (values_y == TRUE) "sum" ~ x + y
         ))

[abhsarma]

         mutate( z = branch(values_z,
             "constant" ~ 5,
             "linear" ~ x + 1,
             "sum" %when% (values_y == TRUE) ~ x + y
         ))

Something like this shouldn't be too difficult to implement from where we are at right now. I am recursively looking at the rhs of the ~ for the %when% but can do that on both sides of ~ I guess.

[mjskay]

do you need to look for it recursively? I would guess that it only really is valid if it is used as the top-level expression right?

Like, this is not valid:

         mutate( z = branch(values_z,
             "constant" ~ 5,
             "linear" ~ x + 1,
             "sum" ~ (x %when% (values_y == TRUE)) + y
         ))

Or am I confused?

[abhsarma]

mutate( z = branch(values_z,
             "constant" ~ 5,
             "linear" ~ x + 1,
             "sum" ~ ((x + y) %when% (values_y == TRUE))
         ))

If someone uses excessive parenthesis, it was giving an error. I just need to find the ast where the top-level call is %when%.

[mjskay]

Oh makes sense! So you unwrap however many parens until you get to non-( call (and maybe braces too?) until the first expression that isn't a ( or { and check for %when%? Anything deeper than that should be an error

[abhsarma]

I can throw the error. But it doesn't right now.

[mjskay]

it probably should throw and error? I don't think it makes sense to let people put it absolutely anywhere in the expression, because the semantics don't really line up with what it looks like: it looks like you're making a subexpression conditional but you're actually making the entire thing conditional.

[abhsarma]

Makes sense. Ok there's a bug: Right now, it just looks past it if it encounters something like this: x %when% (values_y == TRUE)) + y

But not for something like this: x + (y %when% (values_y == TRUE))

Might as well throw the error

[mjskay]

Yeah if you only allow it if %when% is at the top of the tree after unwrapping any sequence of ( or { you should be able to detect both of those

[mjskay]

PS as you collect all these corner cases you should be turning them into tests :-P