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
445 stars 54 forks source link

Type Resolver for Nested Dict doesn't work #19

Closed priyankmav closed 3 years ago

priyankmav commented 3 years ago

Hello zeroSteiner,

I am trying to use the type resolver in the below given method.

user_input = {
    "facts": {
        "abc": False,
        "ghf": 0
    },
    "api_response": {
        "aaa": {
            "type": "derma",
            "value": "high"
        },
        "bbb": {
            "type": "onco",
            "value": "low"
        }
    }

}

pf_dtypes = {
    "facts": rule_engine.DataType.MAPPING, 
    "facts.abc": rule_engine.DataType.BOOLEAN,
    "facts.ghf": rule_engine.DataType.FLOAT,
    "api_response": rule_engine.DataType.MAPPING}
con = rule_engine.Context(type_resolver=rule_engine.type_resolver_from_dict(pf_dtypes))
rule_new = rule_engine.Rule("facts.abc == false", context=con)
print(rule_new.matches(user_input))

I am getting the below error (last few lines of the error)--> engine.py", line 156, in _get_resolver raise errors.AttributeResolutionError(name, object_type, thing=thing) rule_engine.errors.AttributeResolutionError: ('abc', <_MappingDataTypeDef name=MAPPING python_type=dict key_type=UNDEFINED value_type=UNDEFINED >)

I have also tried it with the below type resolver dict. But still get a similar error.

pf_dtypes = {
    "facts": rule_engine.DataType.MAPPING, 
    "facts['abc']": rule_engine.DataType.BOOLEAN,
    "facts['ghf']": rule_engine.DataType.FLOAT,
    "api_response": rule_engine.DataType.MAPPING}

The rule works if i change the rule to "facts['abc'] == false", however, it has no impact of the type resolver whatsoever. If the rule is "facts['abc'] == false" and i change "facts.abc": rule_engine.DataType.BOOLEAN to "facts.abc": rule_engine.DataType.STRING, it doesn't correctly throw the TypeError.

zeroSteiner commented 3 years ago

That's because the types are defined incorrectly. It looks like what you want is:

pf_dtypes = {
    "facts": rule_engine.DataType.MAPPING(key_type=rule_engine.DataType.STRING),
    "api_response": rule_engine.DataType.MAPPING(
        key_type=rule_engine.DataType.STRING,
        value_type=rule_engine.DataType.MAPPING(
            key_type=rule_engine.DataType.STRING, value_type=rule_engine.DataType.MAPPING
        )
    )
}

When defining the types of compound types, you can only define them if they type is consistent (values can be marked as nullable though). So if the keys are all strings as is the case with facts then you can define the key type as such, but the value type needs to remain undefined because there could be strings or floats. api_response is a mapping of strings to mappings with string keys and string responses and is reflected in the second definition. There's some more information on this in the Compound Data Types section of the Getting Started page in the docs.

You can't define the types of specific keys/values in the dictionary because it's not like a struct in C, but rather a dictionary in Python.

priyankmav commented 3 years ago

Thanks for the quick reply zeroSteiner. What I noticed is, even with the above type resolver dict mentioned by you, i am unable to use the accessor operator . in the rule syntax. The below rule throws an error rule_engine.Rule("facts.abc == false", context=con)

rule_engine/engine.py", line 156, in _get_resolver
    raise errors.AttributeResolutionError(name, object_type, thing=thing)
rule_engine.errors.AttributeResolutionError: ('abc', <_MappingDataTypeDef name=MAPPING python_type=dict key_type=STRING value_type=UNDEFINED >)

It only works if i change the syntax to --> rule_engine.Rule("facts['abc'] == false", context=con)

zeroSteiner commented 3 years ago

Well facts['abc'] is the correct syntax for referencing a MAPPING key. The attribute style is left over for compatibility from the old days before MAPPING was directly supported. If the data type is undefined you can use either syntax, again for backwards compatibility but if you define the type then you're using a newer version that supports the getitem syntax using brackets.

I'm not convinced that the attribute syntax should be supported for defined MAPPING objects and I kind of regret maintaining the compatibility because it's ambiguous.

priyankmav commented 3 years ago

Okay, makes sense. Thank you for taking the time out! Appreciate your help. I will close this now.

zeroSteiner commented 3 years ago

I did fix this issue in commit dfd339a53135f9ca0a1e799b2d9731ef06d1dd70 which was just released in v3.1.2 and has been pushed to PyPi.

Keep in mind that if you're using the attribute style syntax on mapping objects, the native attribute will take precedence. For example:

python -m rule_engine.debug_repl --edit-console
edit the 'context' and 'thing' objects as necessary
>>> thing = {'map': {'length': 123}} 
>>> exit()
exiting the edit console...
rule > map
result: 
OrderedDict([('length', Decimal('123'))])
rule > map.length
result: 
Decimal('1')
rule > map['length']
result: 
Decimal('123')
rule > 

I fixed this though because after reviewing the code it did appear to be a bug and I couldn't find a reason that the behavior shouldn't be consistent regardless of whether or not the type was defined.

priyankmav commented 3 years ago

Thank you Spencer! 👍