zeroSteiner / rule-engine

A lightweight, optionally typed expression language with a custom grammar for matching arbitrary Python objects.
https://zerosteiner.github.io/rule-engine/
BSD 3-Clause "New" or "Revised" License
455 stars 54 forks source link

EvaluationError: data type mismatch #83

Closed danisola closed 7 months ago

danisola commented 7 months ago

First of all: congrats on the project, it's the rules one can write with it are very short and elegant!

Now to the issue: I'm puzzled by this EvaluationError triggered with this snippet, which is a variation of the one in the docs:

import rule_engine

context = rule_engine.Context(default_value=None)
rule_engine.Rule('title', context=context).matches({}) # False
rule_engine.Rule('pages > 50', context=context).matches({}) # EvaluationError

Am I missing anything? Shouldn't the 2nd rule also return False?

zeroSteiner commented 7 months ago

First of all: congrats on the project, it's the rules one can write with it are very short and elegant!

Thanks!

Am I missing anything? Shouldn't the 2nd rule also return False?

No because in this case the default value is defined as None (python) or NULL (rule engine). You can't compare NULL to a numeric data type, e.g. 50 in this case. It doesn't make sense. If however you define pages, then it'll evaluate correctly because the default value is not in use.

rule_engine.Rule('pages > 50', context=context).matches({'pages': 100}) # True

Or alternatively, if you set the default value to 0, then you'd be able to compare pages even when undefined to another numeric.

context = rule_engine.Context(default_value=0)
rule_engine.Rule('pages > 50', context=context).matches({}) # False

I hope that helps clarify things.

danisola commented 7 months ago

That makes sense. I was trying to find an example where the rule would fail if two required variables of different types were used:

context = rule_engine.Context(default_value=0)
rule_engine.Rule("pages > 50 or topic == 'fantasy'", context=context).matches({'author': 'Jane'}) # False

I was expecting the same EvaluationError given that the default value of topic would be 0. But it seems that the engine doesn't complain about topic not being passed to the rule.

Given the above, seems that setting the default value to 0 is safer than using None. What I was looking for is a generic way to get a rule to "return False if required variables are missing".

zeroSteiner commented 7 months ago

I wouldn't say that setting the default value to 0 is "safer" but rather that the equality operator is more forgiving than the less than operator, which only works for floats, IIRC.

Really, I wouldn't recommend using the default_value if you have multiple fields of different types that could be missing. Instead I'd suggest you normalize the input that you're matching on.

Something like:

context = rule_engine.Context()
defaults = {'pages': 0, 'topic': 'fantasy'}
rule_engine.Rule("pages > 50 and topic == 'fantasy'", context=context).matches(defaults | {'author': 'Jane'})
danisola commented 7 months ago

That's indeed a better approach. Thanks a lot for the quick and thoughtful replies 👍