Making function calls in rules

[antisvin]

The problem with making function calls now is that clauses get converted to C-expressions, and we can't really define passing args there. A possible solution would be a new define clause that has function definitions.

For instance, let's consider evaluating code like math.sqrt(foo) > 2:

define:
    foo_root: math.sqrt(foo)
if:
    foo_root > 2
...

Basically, function call results would be added to context for evaluating if conditons, but won't be stored in result context (unless we decide that we need them, but I don't see why we'd do that).

[miraculixx]

I'm not sure we're going in the right direction with this. Either we end up implementing a separate parser for the define clause, or we use eval again.

If we go the first path, we're starting to implement an alternative to the eval statement, and I'm not convinced we should do that. At least, I didn't have that in mind when we started with C clauses - the idea was to remove the need for eval (to strictly limit what the user can do), not to re-implement it in different form.

That said, I'm not convinced we even need the complexity of doing so:

In the end, the condition statement foo_root > 2 is the equivalent of math.sqrt(foo) > 2.

Let's assume for a moment we have the property lookup as you suggest in #22, and there is a property sqrt on context.foo which returns the equivalent of calling math.sqrt(foo's value) , then we'd have a valid C expression along the lines of C(foo__sqrt__gt=2). Right?

If we can support that, I don't see a reason why we cannot go one step further and make this an implicit property of foo e.g. as C(foo__fn_sqrt__gt=2), where the __fn_ part introduces a (well known) function to be called on foo.

Once we got this far, we can start adding arbitrary "function handlers", including module resolutions, i.e. fn_math__sqrt, or custom functions that additionally take the context as input, e.g. var__cfn__myfunc, resulting in a call to myfunc(context, var), if we want that.

Then users can implement safe & trusted functions (in python) that can be called in conditions. We can do something similar in then clauses (that was the idea of Aexpressions in one of the comments on #19).

[antisvin]

I'm not sure we're going in the right direction with this. Either we end up implementing a separate parser for the define clause, or we use eval again.

The parser for "define" would be mostly written in #22. The only missing thing is adding function calls, but that's not a problem.

Let's assume for a moment we have the property lookup as you suggest in #22, and there is a property sqrt on context.foo which returns the equivalent of calling math.sqrt(foo's value) , then we'd have a valid C expression along the lines of C(foo__sqrt__gt=2). Right?

Yes, it's possible to use the property workaround where you use object methods with no arguments like you'd do with django templates. I was actually going to mentioned this, but decided I don't like this approach that much. We'll use it anyway initially, since adding function calls is trivial for current code. It may be even sufficient and we don't need this issue that much.

The reason for current proposal was to keep all logic in rule definition, while relying on properties requires writing python code, and foo is no longer a number, but an object with a value and property. So we can't just pass it with HTTP request to default REST API and need a special API for every occasion.

In the end, the condition statement foo_root > 2 is the equivalent of math.sqrt(foo) > 2.

Yes and no. It gives the same result, of course. The difference is that:

1. We don't need to change C-exp's for this to work.
2. math.sqrt(foo) is compiled and evaluated only once for all rules, not every time it's called.

Effectively, this is variable binding to the runtime context.

C(foo__fn_sqrt__gt=2)
var__cfn__myfunc

This starts to look uglier and uglier :-( I think that if we want to have function calls in C-exps, they should be changed to just contain parsed node data. In fact that could be a part of #22. So we'd have something like this C([['call', 'math.sqrt', 'foo'], '>', 5]). Not sure if this a viable approach, but I'll give it some more thought. But C-exps format is just implementation detail, we can change it if necessary.

[miraculixx]

Well, it seems we both have somewhat strong ideas on this, which is a good thing as generally well argued solutions become more solid in the end. So...

The parser for "define" would be mostly written in #22. The only missing thing is adding function
calls, but that's not a problem.

Sure, I realize that - don't get me wrong, I see the merit of the define statement, yet can also see that it takes us way further than the parsing / evaluation of YAML-defined rules was intended.

Once we start that route, it will be hard to stop before we'll have implemented at least a subset of PyPy, because naturally, once we can do function calls, we'll want iterators, soon after that exception handling, context managers, indention/multi-line support, full-blown function definitions, inner functions etc...

At which point it'll have made more sense to simply stick to eval in the first place and execute it in a stripped down container, or use some other ready-made secure/secure-ish python expression evaluator (there are a few around of those). If we need that, and I doubt it, we'd better opt to adopt one of these right away.

The reason for current proposal was to keep all logic in rule definition, while relying on properties requires writing python code, and foo is no longer a number, but an object with a value and property.
So we can't just pass it with HTTP request to default REST API and need a special API for every
occasion.

Indeed, relying on properties should require python code by design. Much like accessing properties in Django templates requires writing python code. I think what is important here is that we do such extensions by using a well defined extension pattern, and that is what the __funcname pattern does.

As far as the REST API goes, rule evaluation should never require more than a serialized context and the name of the ruleset as input.

Rule definitions on the other hand should always be through other means, that is by delivering .egg files which of course can contain python code. We don't need the REST API to support that though (I hope that was clear from #4).

That said, even then we can accept a value with type information attached to it, e.g. context = { 'foo' : { 'value' : 'something', '_type_' : 'SomeClass'} } I'm not advocating this syntax, just as an example. I'm not entirely convinced we should do that either, I was referring to your proposal in #22.

As for using pyrules in an "embedded" mode, i.e. directly using it from within a python application, we are not constrained by the above of course, and intermingling directly written C objects with those generated from a YAML definition should be possible easily.

This starts to look uglier and uglier :-(

well let's see

C(foo__fn_sqrt__gt=2)
var__cfn__myfunc
v.s. 
C([['call', 'math.sqrt', 'foo'], '>', 5])

Frankly, I'm not sure the latter is less ugly ...

Let's regress to the initial proposal, which was along the lines of C(foo__sqrt__gt=2). Note I changed my mind on the place of the funcname bit seeing your adoption of the multi-level "bit__" pattern). I was using C(foo__fn_sqrt__gt=2) to distinguish function calls from the property access example before, but the fn_/cfn_ bits are not really required - it is sufficient to check whether sqrt is a known property or function, and whether passing the context can be made a matter of checking the type of the function (e.g. is it the result of some decorator applied -- again, just cheaply copying from Django here, specifically template tags). As long as we follow a well defined order of lookup. As you mentioned in another context that's not unlike Django's template property lookup works.

Indeed, using the key__[key|func]__op=value pattern follows the same logic as we already have, it is well known by Django's Q objects. Passing a list of a list, operator and value carries the semantic meaning but it sure seems less readable and is geared more more towards an internal format that of course may be efficient if you look at it from the point of the parse tree, but doesn't seem to help the syntax.

[miraculixx]

math.sqrt(foo) is compiled and evaluated only once for all rules, not every time it's called.

not sensible re. evaluation, as rules can rely on other rules and need to see whatever is the current value of foo, see #3.