diff --git a/querybuilder/constants.py b/querybuilder/constants.py index 413b443..946f005 100644 --- a/querybuilder/constants.py +++ b/querybuilder/constants.py @@ -84,6 +84,9 @@ def handles(self, f): f.operator = self return f + +# Does it make sense to just declare these in the class? At the very least, helps with PyCharm's +# jump to definition feature Operators.unary_comparisons = frozenset({ Operators.IS_NULL, Operators.IS_NOT_NULL diff --git a/querybuilder/filters.py b/querybuilder/filters.py index 3f28757..98bc13d 100644 --- a/querybuilder/filters.py +++ b/querybuilder/filters.py @@ -271,10 +271,12 @@ def equal(self, lop, rop): def not_equal(self, lop, rop): return not self.equal(lop, rop) + # Does this belong on the base class? This doesn't fit with scalars @Operators.IN.handles def _in(self, lop, rop): return lop in rop + # Does this belong on the base class? This doesn't fit with scalars @Operators.NOT_IN.handles def not_in(self, lop, rop): return not self._in(lop, rop) @@ -306,6 +308,7 @@ def between(self, op, minop, maxop): def not_between(self, op, minop, maxop): return not self.between(op, minop, maxop) + # Does this belong on the base class? This doesn't fit with scalars @Operators.CONTAINS.handles def contains(self, lop, rop): return self._in(lop, rop) @@ -320,6 +323,7 @@ def is_not_null(self, op): class TypedFilter(Filter): + # Would it make sense to use abstract properties? TYPE = NotImplemented OPERATORS = NotImplemented OPTIONS = NotImplemented @@ -328,6 +332,7 @@ def __init__(self, *args, **kwargs): kwargs.update(type=self.TYPE) assert self.TYPE is not NotImplemented, 'TYPE must be declared in the subclass' + # BUG: If self.OPERATORS if self.OPTIONS is not NotImplemented: kwargs.setdefault('operators', tuple(self.OPERATORS)) @@ -423,7 +428,6 @@ class DateFilter(TypedFilter): # TODO add default validator - class TimeFilter(TypedFilter): TYPE = Types.TIME diff --git a/querybuilder/rules.py b/querybuilder/rules.py index 3c652a4..1a7ee4a 100644 --- a/querybuilder/rules.py +++ b/querybuilder/rules.py @@ -70,11 +70,17 @@ def validate(self, value): pass +# Since you were wanting a better entry point, would it make more sense to have a `RulesEngine` +# class or something similar? You'd pass the json to that and it'd handle making the rules as +# necessary. Then this class doesn't need to worry about serder at all. +# Not that much of an improvement, but at least it's a more obvious entry point. class Rule(object): def __init__(self, rule): ''' Args: + What's the rule object? A dict? rule: The rule object + # Looks like these params have been removed operators: Enum of operators, this allows for adding custom operators inputs: Enum of inputs, this allows for adding custom inputs types: Enum of types, this allows for adding custom types @@ -83,8 +89,9 @@ def __init__(self, rule): self.is_group = False self.is_empty = False # note that an empty rule evaluates as true + # Could you add some more documentation on what these 3 types of rules are? if rule.get('empty'): - # some rules + # some rules what? self.is_empty = True elif 'condition' and 'rules' in rule: self.is_group = True