From fd309457511e78cb8c14a5ef6d01ca64c6ff59ef Mon Sep 17 00:00:00 2001 From: David Rajaratnam Date: Sun, 5 May 2024 12:09:58 +1000 Subject: [PATCH 1/9] Main change to the semantics of Predicate comparison Predicate comparison now uses the underlying clingo.Symbol object. Will allow querying with ordering to work even when the types are incomparable because Symbol objects are always comparable. Still needs to update some of the querying behaviour --- clorm/orm/core.py | 198 +++++++++++++++++-------------------- clorm/orm/templating.py | 95 +++++++++++++++++- tests/support.py | 13 +++ tests/test_orm_core.py | 105 +++++++++++--------- tests/test_orm_factbase.py | 10 +- tests/test_orm_query.py | 8 +- 6 files changed, 266 insertions(+), 163 deletions(-) diff --git a/clorm/orm/core.py b/clorm/orm/core.py index 487fa7f..2e1b588 100644 --- a/clorm/orm/core.py +++ b/clorm/orm/core.py @@ -1,12 +1,21 @@ -# ----------------------------------------------------------------------------- -# Implementation of the core part of the Clorm ORM. In particular this provides -# the base classes and metaclasses for the definition of fields, predicates, -# predicate paths, and the specification of query conditions. Note: query -# condition specification is provided here because the predicate path comparison -# operators are overloads to return these objects. However, the rest of the -# query API is specified with the FactBase and select querying mechanisms -# (see factbase.py). -# ------------------------------------------------------------------------------ +# ------------------------------------------------------------------------------------------- +# Implementation of the core part of the Clorm ORM. In particular this provides the base +# classes and metaclasses for the definition of fields, predicates, predicate paths, and the +# specification of query conditions. Note: query condition specification is provided here +# because the predicate path comparison operators are overloads to return these +# objects. However, the rest of the query API is specified with the FactBase and select +# querying mechanisms (see factbase.py). +# ------------------------------------------------------------------------------------------- + +# ------------------------------------------------------------------------------------------- +# NOTE: 20242028 the semantics for the comparison operators has changed. Instead of using the +# python field representation we use the underlying clingo symbol object. The symbol object is +# well defined for any comparison between symbols, whereas tuples are only well defined if the +# types of the individual parameters are compatible. So this change leads to more natural +# behaviour for the queries. Note: users should avoid defining unintuitive fields (for example +# a swap field that changes the sign of an int) to avoid unintuitive Python behaviour. +# ------------------------------------------------------------------------------------------- + from __future__ import annotations @@ -1341,7 +1350,6 @@ class DateField(StringField): ``FactBase```. Defaults to ``False``. """ - def __init__(self, default: Any = MISSING, index: Any = MISSING) -> None: self._index = index if index is not MISSING else False @@ -1349,33 +1357,35 @@ def __init__(self, default: Any = MISSING, index: Any = MISSING) -> None: self._default = (False, None) return - self._default = (True, default) cmplx = self.complex - # Check and convert the default to a valid value. Note: if the default - # is a callable then we can't do this check because it could break a - # counter type procedure. - if callable(default) or (cmplx and isinstance(default, cmplx)): - return + def _process_basic_value(v): + return v - try: - if cmplx: + def _process_cmplx_value(v): + if isinstance(v, cmplx): + return v + if isinstance(v, tuple) or (isinstance(v, Predicate) and v.meta.is_tuple): + return cmplx(*v) + raise TypeError(f"Value {v} ({type(v)}) cannot be converted to type {cmplx}") - def _instance_from_tuple(v): - if isinstance(v, tuple) or (isinstance(v, Predicate) and v.meta.is_tuple): - return cmplx(*v) - raise TypeError(f"Value {v} ({type(v)}) is not a tuple") + _process_value = _process_basic_value if cmplx is None else _process_cmplx_value + + # If the default is not a factory function than make sure the value can be converted to + # clingo without error. + if not callable(default): + try: + self._default = (True, _process_value(default)) + self.pytocl(self._default[1]) + except (TypeError, ValueError): + raise TypeError( + 'Invalid default value "{}" for {}'.format(default, type(self).__name__) + ) + else: + def _process_default(): + return _process_value(default()) + self._default = (True, _process_default) - if cmplx.meta.is_tuple: - self._default = (True, _instance_from_tuple(default)) - else: - raise ValueError("Bad default") - else: - self.pytocl(default) - except (TypeError, ValueError): - raise TypeError( - 'Invalid default value "{}" for {}'.format(default, type(self).__name__) - ) @staticmethod @abc.abstractmethod @@ -1503,11 +1513,11 @@ def field( raise TypeError(f"{basefield} can just be of Type '{BaseField}' or '{Sequence}'") -# ------------------------------------------------------------------------------ -# RawField is a sub-class of BaseField for storing Symbol or NoSymbol -# objects. The behaviour of Raw with respect to using clingo.Symbol or -# noclingo.NoSymbol is modified by the symbol mode (get_symbol_mode()) -# ------------------------------------------------------------------------------ +# ------------------------------------------------------------------------------------------ +# RawField is a sub-class of BaseField for storing Symbol objects. The behaviour of Raw with +# respect to using clingo.Symbol or noclingo.Symbol is modified by the symbol mode +# (get_symbol_mode()) +# ------------------------------------------------------------------------------------------ class Raw(object): @@ -2441,13 +2451,9 @@ def get_field_definition(defn: Any, module: str = "") -> BaseField: def _create_complex_term(defn: Any, default_value: Any = MISSING, module: str = "") -> BaseField: - # NOTE: I was using a dict rather than OrderedDict which just happened to - # work. Apparently, in Python 3.6 this was an implmentation detail and - # Python 3.7 it is a language specification (see: - # https://stackoverflow.com/questions/1867861/how-to-keep-keys-values-in-same-order-as-declared/39537308#39537308). - # However, since Clorm is meant to be Python 3.5 compatible change this to - # use an OrderedDict. - # proto = { "arg{}".format(i+1) : get_field_definition(d) for i,d in enumerate(defn) } + # NOTE: relies on a dict preserving insertion order - this is true from Python 3.7+. Python + # 3.7 is already end-of-life so there is no longer a reason to use OrderedDict. + #proto = {f"arg{idx+1}": get_field_definition(dn) for idx, dn in enumerate(defn)} proto: Dict[str, Any] = collections.OrderedDict( [(f"arg{i+1}", get_field_definition(d, module)) for i, d in enumerate(defn)] ) @@ -2705,13 +2711,16 @@ def _generate_dynamic_predicate_functions(class_name: str, namespace: Dict) -> N gdict = { "Predicate": Predicate, + "Symbol": Symbol, "Function": Function, "MISSING": MISSING, "AnySymbol": AnySymbol, "Type": Type, + "Any": Any, "Optional": Optional, "Sequence": Sequence, "_P": _P, + "PREDICATE_IS_TUPLE": pdefn.is_tuple, } for f in pdefn: @@ -2786,21 +2795,26 @@ def _generate_dynamic_predicate_functions(class_name: str, namespace: Dict) -> N template = PREDICATE_TEMPLATE.format(pdefn=pdefn) predicate_functions = expand_template(template, **expansions) - # print(f"INIT:\n\n{predicate_functions}\n\n") +# print(f"INIT {class_name}:\n\n{predicate_functions}\n\n") ldict: Dict[str, Any] = {} exec(predicate_functions, gdict, ldict) - init_doc_args = f"{args_signature}*, sign=True, raw=None" - predicate_init = ldict["__init__"] - predicate_init.__name__ = "__init__" - predicate_init.__doc__ = f"{class_name}({init_doc_args})" - predicate_unify = ldict["_unify"] - predicate_unify.__name__ = "_unify" - predicate_unify.__doc__ = PREDICATE_UNIFY_DOCSTRING + def _set_fn(fname: str, docstring: str): + tmp = ldict[fname] + tmp.__name__ = fname + tmp.__doc = docstring + namespace[fname] = tmp - namespace["__init__"] = predicate_init - namespace["_unify"] = predicate_unify + # Assign the __init__, _unify, __hash__, and appropriate comparison functions + _set_fn("__init__", f"{class_name}({args_signature}*, sign=True, raw=None)") + _set_fn("_unify", PREDICATE_UNIFY_DOCSTRING) + _set_fn("__hash__", "Hash operator") + _set_fn("__eq__", "Equality operator") + _set_fn("__lt__", "Less than operator") + _set_fn("__le__", "Less than or equal operator") + _set_fn("__gt__", "Greater than operator") + _set_fn("__ge__", "Greater than operator") # ------------------------------------------------------------------------------ @@ -3160,6 +3174,7 @@ def _cltopy(v): # ------------------------------------------------------------------------------ # A Metaclass for the Predicate base class # ------------------------------------------------------------------------------ + @__dataclass_transform__(field_descriptors=(field,)) class _PredicateMeta(type): if TYPE_CHECKING: @@ -3244,8 +3259,12 @@ def __iter__(self) -> Iterator[PredicatePath]: # underlying Symbol object. # ------------------------------------------------------------------------------ +# Mixin class to be able to use both MetaClasses +class _AbstractPredicateMeta(abc.ABCMeta, _PredicateMeta): + pass + -class Predicate(object, metaclass=_PredicateMeta): +class Predicate(object, metaclass=_AbstractPredicateMeta): """Abstract base class to encapsulate an ASP predicate or complex term. This is the heart of the ORM model for defining the mapping of a predicate @@ -3324,7 +3343,7 @@ def _unify( def symbol(self): """Returns the Symbol object corresponding to the fact. - The type of the object maybe either a ``clingo.Symbol`` or ``noclingo.NoSymbol``. + The type of the object maybe either a ``clingo.Symbol`` or ``noclingo.Symbol``. """ return self._raw @@ -3413,74 +3432,35 @@ def __neg__(self): # -------------------------------------------------------------------------- # Overloaded operators # -------------------------------------------------------------------------- + @abc.abstractmethod def __eq__(self, other): """Overloaded boolean operator.""" - if isinstance(other, self.__class__): - return self._field_values == other._field_values and self._sign == other._sign - if self.meta.is_tuple: - return self._field_values == other - elif isinstance(other, Predicate): - return False - return NotImplemented + raise NotImplementedError("Predicate.__eq__() must be overriden") + @abc.abstractmethod def __lt__(self, other): """Overloaded boolean operator.""" + raise NotImplementedError("Predicate.__lt__() must be overriden") - # If it is the same predicate class then compare the sign and fields - if isinstance(other, self.__class__): - - # Negative literals are less than positive literals - if self.sign != other.sign: - return self.sign < other.sign - - return self._field_values < other._field_values - - # If different predicates then compare the raw value - elif isinstance(other, Predicate): - return self.raw < other.raw - - # Else an error - return NotImplemented + def __le__(self, other): + """Overloaded boolean operator.""" + raise NotImplementedError("Predicate.__le__() must be overriden") + @abc.abstractmethod def __ge__(self, other): """Overloaded boolean operator.""" - result = self.__lt__(other) - if result is NotImplemented: - return NotImplemented - return not result + raise NotImplementedError("Predicate.__ge__() must be overriden") + @abc.abstractmethod def __gt__(self, other): """Overloaded boolean operator.""" + raise NotImplementedError("Predicate.__gt__() must be overriden") - # If it is the same predicate class then compare the sign and fields - if isinstance(other, self.__class__): - # Positive literals are greater than negative literals - if self.sign != other.sign: - return self.sign > other.sign - - return self._field_values > other._field_values - - # If different predicates then compare the raw value - if not isinstance(other, Predicate): - return self.raw > other.raw - - # Else an error - return NotImplemented - - def __le__(self, other): - """Overloaded boolean operator.""" - result = self.__gt__(other) - if result is NotImplemented: - return NotImplemented - return not result + @abc.abstractmethod def __hash__(self): - if self._hash is None: - if self.meta.is_tuple: - self._hash = hash(self._field_values) - else: - self._hash = hash((self.meta.name, self._field_values)) - return self._hash + """Overload the hash function.""" + raise NotImplementedError("Predicate.__hash__() must be overriden") def __str__(self): """Returns the Predicate as the string representation of an ASP fact.""" diff --git a/clorm/orm/templating.py b/clorm/orm/templating.py index 8c4c4c3..287c8f0 100644 --- a/clorm/orm/templating.py +++ b/clorm/orm/templating.py @@ -37,16 +37,16 @@ def add_spaces(num, text): lines = template.expandtabs(4).splitlines() outlines = [] for line in lines: - start = line.find("{%") + start = line.find(r"{%") if start == -1: outlines.append(line) continue - end = line.find("%}", start) + end = line.find(r"%}", start) if end == -1: raise ValueError("Bad template expansion in {line}") - keyword = line[start + 2 : end] + keyword = line[start + 2:end] text = add_spaces(start, kwargs[keyword]) - line = line[0:start] + text + line[end + 2 :] + line = line[0:start] + text + line[end + 2:] outlines.append(line) return "\n".join(outlines) @@ -73,6 +73,7 @@ def __init__(self, ({{%args_raw%}}), self._sign) + @classmethod def _unify(cls: Type[_P], raw: AnySymbol, raw_args: Optional[Sequence[AnySymbol]]=None, raw_name: Optional[str]=None) -> Optional[_P]: try: @@ -98,6 +99,92 @@ def _unify(cls: Type[_P], raw: AnySymbol, raw_args: Optional[Sequence[AnySymbol] raise ValueError((f"Cannot unify with object {{raw}} ({{type(raw)}}) as " "it is not a clingo Symbol Function object")) + +def nontuple__eq__(self, other: Any) -> bool: + # Deal with a non-tuple predicate + if isinstance(other, Predicate): + return self._raw == other._raw + if isinstance(other, Symbol): + return self._raw == other + return NotImplemented + + +def tuple__eq__(self, other: Any) -> bool: + # Deal with a predicate that is a tuple + if isinstance(other, Predicate): + return self._raw == other._raw + if isinstance(other, Symbol): + return self._raw == other +# if isinstance(other, tuple): +# return self._field_values == other + return NotImplemented + + +def nontuple__lt__(self, other): + # If it is the same predicate class then compare the underlying clingo symbol + if isinstance(other, Predicate): + return self._raw < other._raw + if isinstance(other, Symbol): + return self._raw < other + return NotImplemented + + +def tuple__lt__(self, other): + # self is always less than a non-tuple predicate + if isinstance(other, Predicate): + return self._raw < other._raw + if isinstance(other, Symbol): + return self._raw < other +# if isinstance(other, tuple): +# return self._field_values < other + return NotImplemented + + +def nontuple__gt__(self, other): + if isinstance(other, Predicate): + return self._raw > other._raw + if isinstance(other, Symbol): + return self._raw > other + return NotImplemented + + +def tuple__gt__(self, other): + # If it is the same predicate class then compare the sign and fields + if isinstance(other, Predicate): + return self._raw > other._raw + if isinstance(other, Symbol): + return self._raw > other +# if isinstance(other, tuple): +# return self._field_values > other + return NotImplemented + + +def __ge__(self, other): + result = self.__lt__(other) + if result is NotImplemented: + return NotImplemented + return not result + + +def __le__(self, other): + result = self.__gt__(other) + if result is NotImplemented: + return NotImplemented + return not result + + +def __hash__(self): + if self._hash is None: + self._hash = hash(self._raw) + return self._hash + + +__eq__ = tuple__eq__ if PREDICATE_IS_TUPLE else nontuple__eq__ +__lt__ = tuple__lt__ if PREDICATE_IS_TUPLE else nontuple__lt__ +__gt__ = tuple__gt__ if PREDICATE_IS_TUPLE else nontuple__gt__ + + + """ CHECK_SIGN_TEMPLATE = r""" diff --git a/tests/support.py b/tests/support.py index d3fd022..2ce3be6 100644 --- a/tests/support.py +++ b/tests/support.py @@ -1,3 +1,5 @@ +from clorm import Predicate + # ------------------------------------------------------------------------------ # Support functions for the unit tests # ------------------------------------------------------------------------------ @@ -19,6 +21,17 @@ def check_errmsg_contains(contmsg, ctx): raise AssertionError(msg) +# ------------------------------------------------------------------------------ +# +# ------------------------------------------------------------------------------ + +def to_tuple(value): + """Recursively convert a predicate/normal tuple into a Python tuple""" + if isinstance(value, tuple) or (isinstance(value, Predicate) and value.meta.is_tuple): + return tuple(to_tuple(x) for x in value) + return value + + # ------------------------------------------------------------------------------ # # ------------------------------------------------------------------------------ diff --git a/tests/test_orm_core.py b/tests/test_orm_core.py index aa24145..469a5f7 100644 --- a/tests/test_orm_core.py +++ b/tests/test_orm_core.py @@ -8,6 +8,12 @@ # to be completed. # ------------------------------------------------------------------------------ +# ------------------------------------------------------------------------------------------- +# NOTE: 20242028 See orm/core.py for changes to the semantics of comparison operators for +# Predicate objects. +# ------------------------------------------------------------------------------------------- + + import collections.abc as cabc import datetime import enum @@ -71,7 +77,7 @@ trueall, ) -from .support import check_errmsg, check_errmsg_contains +from .support import check_errmsg, check_errmsg_contains, to_tuple # Error messages for CPython and PyPy vary PYPY = sys.implementation.name == "pypy" @@ -280,10 +286,12 @@ def test_api_field_function(self): self.assertEqual(t, (StringField, IntegerField)) t = field((StringField, IntegerField), default=("3", 4)) + self.assertIsInstance(t, BaseField) self.assertIsInstance(t.complex[0].meta.field, StringField) self.assertIsInstance(t.complex[1].meta.field, IntegerField) - self.assertEqual(t.default, ("3", 4)) + self.assertEqual(t.default, t.complex("3", 4)) + self.assertEqual(to_tuple(t.default), ("3", 4)) with self.subTest("with custom field"): INLField = define_flat_list_field(IntegerField, name="INLField") @@ -302,8 +310,8 @@ def factory(): return ("3", x) t = field((StringField, IntegerField), default_factory=factory) - self.assertEqual(t.default, ("3", 1)) - self.assertEqual(t.default, ("3", 2)) + self.assertEqual(to_tuple(t.default), ("3", 1)) + self.assertEqual(to_tuple(t.default), ("3", 2)) with self.subTest("with nested tuple and default"): t = field((StringField, (StringField, IntegerField))) @@ -313,7 +321,7 @@ def factory(): self.assertIsInstance(t, BaseField) self.assertIsInstance(t.complex[0].meta.field, StringField) self.assertIsInstance(t.complex[1].meta.field, BaseField) - self.assertEqual(t.default, ("3", ("1", 4))) + self.assertEqual(to_tuple(t.default), ("3", ("1", 4))) def test_api_field_function_illegal_arguments(self): with self.subTest("illegal basefield type"): @@ -783,7 +791,7 @@ def test_api_nested_list_field_complex_element_field(self): value2 = (2, ("b", "B")) nlist = (value1, value2) - self.assertEqual(XField.cltopy(symnlist), nlist) + self.assertEqual(to_tuple(XField.cltopy(symnlist)), nlist) self.assertEqual(XField.pytocl(nlist), symnlist) # -------------------------------------------------------------------------- @@ -835,7 +843,7 @@ def test_api_flat_list_field_complex_element_field(self): value2 = (2, ("b", "B")) nlist = (value1, value2) - self.assertEqual(XField.cltopy(symnlist), nlist) + self.assertEqual(to_tuple(XField.cltopy(symnlist)), nlist) self.assertEqual(XField.pytocl(nlist), symnlist) # -------------------------------------------------------------------------- @@ -1014,6 +1022,25 @@ class Blah(object): clresult = td.pytocl((1, "blah")) self.assertEqual(clresult, clob) + + # -------------------------------------------------------------------------- + # Test that we define the new comparison operators in the template + # -------------------------------------------------------------------------- + def test_predicate_comparison_operator_creation(self): + + class P(Predicate, name="p"): + a = IntegerField + b = ConstantField + + p1 = P(a=1, b="x") + p2 = P(a=1, b="x") + p3 = P(a=2, b="x") + + tmp = {} + tmp[p1] = "P" + self.assertEqual(p1, p2) + self.assertNotEqual(p1, p3) + # -------------------------------------------------------------------------- # Test that we can define predicates using the class syntax and test that # the getters and setters are connected properly to the predicate classes. @@ -1177,21 +1204,25 @@ class T(Predicate): tuple1 = tuple([1, "a"]) tuple2 = tuple([2, "b"]) - # Equality works even when the types are different + # Equality works even when the predicate types are different self.assertTrue(p1.tuple_ == p1_alt.tuple_) self.assertTrue(p1.tuple_ == q1.tuple_) self.assertTrue(p1.tuple_ == t1.tuple_) - self.assertTrue(p1.tuple_ == tuple1) + + # New behaviour. Doesn't compare directly to tuple + self.assertFalse(p1.tuple_ == tuple1) + self.assertTrue(tuple(p1.tuple_) == tuple1) self.assertNotEqual(type(p1.tuple_), type(t1.tuple_)) - # self.assertNotEqual(type(p1.tuple_), type(t1)) self.assertTrue(p1.tuple_ != p2.tuple_) self.assertTrue(p1.tuple_ != q2.tuple_) self.assertTrue(p1.tuple_ != r2.tuple_) self.assertTrue(p1.tuple_ != s2.tuple_) self.assertTrue(p1.tuple_ != t2.tuple_) - self.assertTrue(p1.tuple_ != tuple2) + + self.assertTrue(p1.tuple_ != tuple1) + self.assertFalse(tuple(p1.tuple_) != tuple1) # -------------------------------------------------------------------------- # Test predicates with default fields @@ -2113,33 +2144,18 @@ def test_predicate_comparison_operator_overload_signed(self): class P(Predicate): a = IntegerField - class Q(Predicate): - a = IntegerField - p1 = P(1) neg_p1 = P(1, sign=False) p2 = P(2) neg_p2 = P(2, sign=False) - q1 = Q(1) - - self.assertTrue(neg_p1 < neg_p2) - self.assertTrue(neg_p1 < p1) - self.assertTrue(neg_p1 < p2) - self.assertTrue(neg_p2 < p1) - self.assertTrue(neg_p2 < p2) - self.assertTrue(p1 < p2) - - self.assertTrue(p2 > p1) - self.assertTrue(p2 > neg_p2) - self.assertTrue(p2 > neg_p1) - self.assertTrue(p1 > neg_p2) - self.assertTrue(p1 > neg_p1) - self.assertTrue(neg_p2 > neg_p1) - # Different predicate sub-classes are incomparable + # NOTE: 20240428 see note at top about change of semantics + self.assertTrue((neg_p1.raw < p1.raw) == (neg_p1 < p1)) + self.assertTrue((neg_p1.raw < p2.raw) == (neg_p1 < p2)) + self.assertTrue((neg_p2.raw < p1.raw) == (neg_p2 < p1)) + self.assertTrue((neg_p2.raw < p2.raw) == (neg_p2 < p2)) + self.assertTrue((p1.raw < p2.raw) == (p1 < p2)) - # with self.assertRaises(TypeError) as ctx: - # self.assertTrue(p1 < q1) # -------------------------------------------------------------------------- # Test a simple predicate with a field that has a function default @@ -2320,9 +2336,9 @@ class Fact(Predicate): with self.assertRaises(TypeError) as ctx: class Fact2(Predicate): - afun = Fun.Field(default=(1, "str")) + afun = Fun.Field(default=6) - check_errmsg("""Invalid default value "(1, 'str')" for FunField""", ctx) + check_errmsg("""Invalid default value "6" for FunField""", ctx) # -------------------------------------------------------------------------- # Test the simple_predicate function as a mechanism for defining @@ -2423,6 +2439,7 @@ class Fact(Predicate): # Test predicate equality # -------------------------------------------------------------------------- def test_predicate_comparison_operator_overloads(self): + # NOTE: 20240428 see note at top about change of semantics f1 = Function("fact", [Number(1)]) f2 = Function("fact", [Number(2)]) @@ -2452,9 +2469,9 @@ class Meta: self.assertEqual(f1, af1.raw) self.assertEqual(af1.raw, f1) self.assertEqual(af1.raw, ag1.raw) - self.assertNotEqual(af1, ag1) - self.assertNotEqual(af1, f1) - self.assertNotEqual(f1, af1) + self.assertEqual(af1, ag1) + self.assertEqual(af1, f1) + self.assertEqual(f1, af1) self.assertTrue(af1 < af2) self.assertTrue(af1 <= af2) @@ -2478,6 +2495,8 @@ class Meta: # Test predicate equality # -------------------------------------------------------------------------- def test_comparison_operator_overloads_complex(self): + # NOTE: 20240428 see note at top about change of semantics + class SwapField(IntegerField): pytocl = lambda x: 100 - x cltopy = lambda x: 100 - x @@ -2497,17 +2516,15 @@ class AComplex(ComplexTerm): for rf in [rf1, rf2, rf3]: self.assertEqual(rf.arguments[0], rf.arguments[1]) - # Test the the comparison operator for the complex term is using the - # swapped values so that the comparison is opposite to what the raw - # field says. self.assertTrue(rf1 < rf2) self.assertTrue(rf2 < rf3) - self.assertTrue(f1 > f2) - self.assertTrue(f2 > f3) - self.assertTrue(f2 < f1) - self.assertTrue(f3 < f2) + self.assertTrue(f1 < f2) + self.assertTrue(f2 < f3) + self.assertTrue(f2 > f1) + self.assertTrue(f3 > f2) self.assertEqual(f3, f4) + # -------------------------------------------------------------------------- # Test unifying a symbol with a predicate # -------------------------------------------------------------------------- diff --git a/tests/test_orm_factbase.py b/tests/test_orm_factbase.py index 15f4be6..9710e6b 100644 --- a/tests/test_orm_factbase.py +++ b/tests/test_orm_factbase.py @@ -1118,6 +1118,9 @@ class Afact(Predicate): # Test that select works with order_by for complex term # -------------------------------------------------------------------------- def test_api_factbase_select_order_by_complex_term(self): + # NOTE: behavior change 20240428 - ordering is based on the underlying clingo.Symbol + # object and not the python translation. So using SwapField won't change the ordering + # for AComplex objects. class SwapField(IntegerField): pytocl = lambda x: 100 - x cltopy = lambda x: 100 - x @@ -1145,10 +1148,13 @@ class AFact(Predicate): self.assertEqual([f1, f2, f3, f4], list(q.get())) q = fb.select(AFact).order_by(AFact.cmplx, AFact.astr) - self.assertEqual([f3, f4, f2, f1], list(q.get())) + self.assertEqual([f1, f2, f3, f4], list(q.get())) q = fb.select(AFact).where(AFact.cmplx <= ph1_).order_by(AFact.cmplx, AFact.astr) - self.assertEqual([f3, f4, f2], list(q.get(cmplx2))) + self.assertEqual([f1, f2], list(q.get(cmplx2))) + + q = fb.select(AFact).where(AFact.cmplx >= ph1_).order_by(AFact.cmplx, AFact.astr) + self.assertEqual([f2, f3, f4], list(q.get(cmplx2))) # -------------------------------------------------------------------------- # Test that select works with order_by for complex term diff --git a/tests/test_orm_query.py b/tests/test_orm_query.py index 46d0b93..a1810db 100644 --- a/tests/test_orm_query.py +++ b/tests/test_orm_query.py @@ -84,7 +84,7 @@ where_expression_to_nnf, ) -from .support import check_errmsg, check_errmsg_contains +from .support import check_errmsg, check_errmsg_contains, to_tuple ###### NOTE: The QueryOutput tests need to be turned into QueryExecutor ###### tests. We can then delete QueryOutput which is not being used for @@ -432,7 +432,7 @@ class F(Predicate): getter = make_input_alignment_functor([F], [F.acomplex]) result = getter((f1,)) tmp = ((1, 2),) - self.assertEqual(result, tmp) + self.assertEqual(to_tuple(result), tmp) # ------------------------------------------------------------------------------ # ------------------------------------------------------------------------------ @@ -545,9 +545,9 @@ class F(Predicate): getter = make_input_alignment_functor([F], [F.acomplex]) result = getter((f1,)) - self.assertEqual(result, ((1, 2),)) + self.assertEqual(to_tuple(result), ((1, 2),)) - sc = SC(operator.eq, [F.acomplex, (1, 2)]) + sc = SC(operator.eq, [F.acomplex, F.meta[1].defn.complex(1, 2)]) cmp = sc.make_callable([F]) self.assertTrue(cmp((f1,))) self.assertFalse(cmp((f2,))) From 2f0dc214f682bd690dc0a96a3f834b84dc13202d Mon Sep 17 00:00:00 2001 From: David Rajaratnam Date: Mon, 6 May 2024 18:31:23 +1000 Subject: [PATCH 2/9] Updated comparison semantics - passing all tests --- clorm/orm/core.py | 24 +++++----- clorm/orm/query.py | 25 +++++++++- tests/test_orm_core.py | 28 +++++++++++ tests/test_orm_factbase.py | 98 +++++++++++++++++++++++++++++++++++++- 4 files changed, 160 insertions(+), 15 deletions(-) diff --git a/clorm/orm/core.py b/clorm/orm/core.py index 2e1b588..0746a56 100644 --- a/clorm/orm/core.py +++ b/clorm/orm/core.py @@ -729,6 +729,14 @@ def hashable(self): def is_leaf(self): return not hasattr(self, "_predicate_class") + # -------------------------------------------------------------------------- + # If the leaf of the path is a Predicate class then return it else None + # -------------------------------------------------------------------------- + @property + def complex(self): + fld = self._parent._get_field() + return None if fld is None else fld.complex + # -------------------------------------------------------------------------- # attrgetter # -------------------------------------------------------------------------- @@ -2453,10 +2461,7 @@ def get_field_definition(defn: Any, module: str = "") -> BaseField: def _create_complex_term(defn: Any, default_value: Any = MISSING, module: str = "") -> BaseField: # NOTE: relies on a dict preserving insertion order - this is true from Python 3.7+. Python # 3.7 is already end-of-life so there is no longer a reason to use OrderedDict. - #proto = {f"arg{idx+1}": get_field_definition(dn) for idx, dn in enumerate(defn)} - proto: Dict[str, Any] = collections.OrderedDict( - [(f"arg{i+1}", get_field_definition(d, module)) for i, d in enumerate(defn)] - ) + proto = {f"arg{idx+1}": get_field_definition(dn) for idx, dn in enumerate(defn)} class_name = ( f'ClormAnonTuple({",".join(f"{arg[0]}={repr(arg[1])}" for arg in proto.items())})' ) @@ -3027,9 +3032,8 @@ def _make_predicatedefn( reserved = set(["meta", "raw", "clone", "sign", "Field"]) - # Generate the fields - NOTE: this relies on dct being an OrderedDict() - # which is true from Python 3.5+ (see PEP520 - # https://www.python.org/dev/peps/pep-0520/) + # Generate the fields - NOTE: this relies on dict being an ordered which is true from + # Python 3.5+ (see PEP520 https://www.python.org/dev/peps/pep-0520/) # Predicates (or complexterms) that are defined within the current Predicate context may be # necessary to help resolve the postponed annotations. @@ -3527,10 +3531,8 @@ def simple_predicate( """ subclass_name = name if name else "AnonSimplePredicate" - # Use an OrderedDict to ensure the correct order of the field arguments - proto: Dict[str, Any] = collections.OrderedDict( - [("arg{}".format(i + 1), RawField()) for i in range(0, arity)] - ) + # Note: dict is preserves the ordering + proto: Dict[str, Any] = {f"arg{i+1}": RawField() for i in range(0, arity)} proto["Meta"] = type( "Meta", (object,), {"name": predicate_name, "is_tuple": False, "_anon": True} ) diff --git a/clorm/orm/query.py b/clorm/orm/query.py index 624a41a..9ffdc19 100644 --- a/clorm/orm/query.py +++ b/clorm/orm/query.py @@ -514,6 +514,27 @@ def membership_op_keyable(sc, indexes): # is not exposed outside clorm. # ------------------------------------------------------------------------------ +# Helper function to try to convert Python tuples into a matching clorm anon tuple. With this +# we can pass a Python tuple to a query and not have to overload the Predicate comparison +# operator and hash function to match the tuple. +def _try_to_convert_tuple_argument_to_clorm_tuple(op, args): + def _try_to_convert(tup, possiblepath): + if not isinstance(possiblepath, PredicatePath): + return tup + complex = possiblepath.meta.complex + if complex is None or not complex.meta.is_tuple: + return tup + return complex(*tup) + + if op not in {operator.eq, operator.ne, operator.lt, operator.le, operator.gt, operator.ge}: + return args + if len(args) != 2: + return args + if isinstance(args[0], tuple): + return (_try_to_convert(args[0], args[1]), args[1]) + if isinstance(args[1], tuple): + return (args[0], _try_to_convert(args[1], args[0])) + return args class StandardComparator(Comparator): class Preference(enum.IntEnum): @@ -625,7 +646,7 @@ def __init__(self, operator, args): ).format(operator) ) self._operator = operator - self._args = tuple(args) + self._args = tuple(_try_to_convert_tuple_argument_to_clorm_tuple(operator, args)) self._hashableargs = tuple( [hashable_path(a) if isinstance(a, PredicatePath) else a for a in self._args] ) @@ -789,7 +810,7 @@ def swap(self): self._operator ) ) - return StandardComparator(spec.swapop, reversed(self._args)) + return StandardComparator(spec.swapop, tuple(reversed(self._args))) def keyable(self, indexes): spec = StandardComparator.operators[self._operator] diff --git a/tests/test_orm_core.py b/tests/test_orm_core.py index 469a5f7..83520dd 100644 --- a/tests/test_orm_core.py +++ b/tests/test_orm_core.py @@ -1224,6 +1224,34 @@ class T(Predicate): self.assertTrue(p1.tuple_ != tuple1) self.assertFalse(tuple(p1.tuple_) != tuple1) + + # -------------------------------------------------------------------------- + # Testing comparison between tuple fields where the corresponding python tuples + # can contain incomparible objects. + # -------------------------------------------------------------------------- + def test_predicate_with_incomparable_python_tuples(self): + class P(Predicate): + tuple_ = field((SimpleField, SimpleField)) + + class Q(Predicate): + tuple_ = field((IntegerField, IntegerField)) + + ptuple = P.tuple_.meta.complex + qtuple = Q.tuple_.meta.complex + + self.assertTrue(ptuple(1, 2) == qtuple(1, 2)) + self.assertTrue(ptuple("a", "b") != qtuple(1, 2)) + + # NOTE: the following throws a TypeError when comparing two Python tuples but works + # with clorm tuples. + # error: self.assertTrue(("a", "b") > (1,2)) + self.assertTrue(ptuple("a", "b") > qtuple(1, 2)) + self.assertTrue(ptuple("a", "b") >= qtuple(1, 2)) + self.assertFalse(ptuple("a", "b") < qtuple(1, 2)) + self.assertFalse(ptuple("a", "b") <= qtuple(1, 2)) + + + # -------------------------------------------------------------------------- # Test predicates with default fields # -------------------------------------------------------------------------- diff --git a/tests/test_orm_factbase.py b/tests/test_orm_factbase.py index 9710e6b..aa3491e 100644 --- a/tests/test_orm_factbase.py +++ b/tests/test_orm_factbase.py @@ -19,6 +19,7 @@ ConstantField, FactBase, IntegerField, + SimpleField, Predicate, StringField, alias, @@ -27,6 +28,7 @@ func, hashable_path, in_, + notin_, path, ph1_, ph2_, @@ -47,6 +49,7 @@ "FactBaseTestCase", "QueryAPI1TestCase", "QueryAPI2TestCase", + "QueryWithTupleTestCase", "SelectJoinTestCase", "MembershipQueriesTestCase", "FactBasePicklingTestCase", @@ -1284,8 +1287,7 @@ class Fact(Predicate): self.assertEqual(list(s1.get(20)), [f2, f1]) self.assertEqual(list(s2.get(CT(20, "b"))), [f2]) - # NOTE: Important test as it requires tuple complex terms to have the - # same hash as the corresponding python tuple. + # NOTE: This requires Python tuple to be converted to a clorm tuple. self.assertEqual(list(s3.get((1, 2))), [f2]) self.assertEqual(list(s4.get((2, 1))), [f2]) @@ -1697,6 +1699,98 @@ def test_api_bad_join_statement(self): check_errmsg("A query over multiple predicates is incomplete", ctx) +# ------------------------------------------------------------------------------------------ +# Test some special cases involving passing a Python tuple instead of using the clorm tuple. +# ------------------------------------------------------------------------------------------ + +class QueryWithTupleTestCase(unittest.TestCase): + def setUp(self): + class F(Predicate): + atuple = field((IntegerField, StringField)) + other = StringField + + class G(Predicate): + atuple = field((SimpleField, StringField)) + other = StringField + + self.F = F + self.G = G + + self.factbase = FactBase( + [ + F((1, "a"), "i"), F((2, "b"), "j"), F((3, "a"), "k"), + G(("a", "a"), "x"), G((2, "b"), "y"), G(("e", "e"), "z") + ] + ) + + + # -------------------------------------------------------------------------- + # Some tuple predicate instance comparisons + # -------------------------------------------------------------------------- + def test_basic_clorm_tuple_in_comparison(self): + G = self.G + fb = self.factbase + cltuple = G.atuple.meta.complex + expected = [G((2, "b"), "y"), G(("a", "a"), "x"), ] + + # NOTE: the version with python tuples fails to find the matching tuples because we can + # no longer directly compare python tuples with clorm tuples. + + pytuples = {("a", "a"), (2, "b")} + q1 = fb.query(G).where(in_(G.atuple, pytuples)).ordered() + self.assertEqual(list(q1.all()), []) + # this fails: self.assertEqual(list(q1.all()), expected) + + cltuples = {cltuple("a", "a"), cltuple(2, "b")} + q2 = fb.query(G).where(in_(G.atuple, cltuples)).ordered() + self.assertEqual(list(q2.all()), expected) + + + # -------------------------------------------------------------------------- + # Complex query where the join is on a tuple object + # -------------------------------------------------------------------------- + def test_api_join_on_clorm_tuple(self): + F = self.F + G = self.G + fb = self.factbase + + # Select everything with an equality join + q = fb.query(G, F).join(F.atuple == G.atuple).where(F.other == "j") + expected = [(G((2, "b"), "y"), F((2, "b"), "j"))] + self.assertEqual(list(q.all()), expected) + + + # -------------------------------------------------------------------------- + # Complex query with a where containing a tuple + # -------------------------------------------------------------------------- + def test_api_where_with_python_tuple(self): + F = self.F + fb = self.factbase + + # The Python tuple passed in the where clause + q = fb.query(F).where(F.atuple == (2, "b")) + expected = [F((2, "b"), "j")] + self.assertEqual(list(q.all()), expected) + + # The Python tuple passed in the bind + q = fb.query(F).where(F.atuple == ph1_).bind((2, "b")) + expected = [F((2, "b"), "j")] + self.assertEqual(list(q.all()), expected) + + # -------------------------------------------------------------------------- + # Complex query sorting on Clorm tuple where the corresponding Python tuple + # is incomparable. + # -------------------------------------------------------------------------- + def test_api_sorting_with_incomparable_elements(self): + G = self.G + fb = self.factbase + + q = fb.query(G).order_by(G.atuple) + expected = [ + G((2, "b"), "y"), G(("a", "a"), "x"), G(("e", "e"), "z") + ] + self.assertEqual(list(q.all()), expected) + # ------------------------------------------------------------------------------ # Tests for additional V2 select join statements # ------------------------------------------------------------------------------ From b7dc53d366ee1a1744cbb211d5d51af8bb3a7685 Mon Sep 17 00:00:00 2001 From: David Rajaratnam Date: Mon, 6 May 2024 21:34:09 +1000 Subject: [PATCH 3/9] Update documentation for comparison operator change --- docs/clorm/predicate.rst | 75 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 69 insertions(+), 6 deletions(-) diff --git a/docs/clorm/predicate.rst b/docs/clorm/predicate.rst index 2c1d04f..e1013b5 100644 --- a/docs/clorm/predicate.rst +++ b/docs/clorm/predicate.rst @@ -109,10 +109,11 @@ There are some things to note here: * The use of a *default value*: all term types support the specification of a default value. -* It is also possible to specify a *default factory* that is used to generate values. This must - be a unary function (i.e., called with no arguments) that is called when the - predicate/complex-term object is instantiated. This can be used to generated unique ids or a - date/time stamp. +* It is also possible to specify a *default factory* function, using the `default_factory` + parameter for the :py:func:`~clorm.field` function. This must be a unary function (i.e., + called with no arguments) that is called when the predicate/complex-term object is + instantiated. This can be used to generated unique ids or a date/time stamp. + Overriding the Predicate Name ----------------------------- @@ -646,7 +647,7 @@ above fact can be defined simply (using the ``DateField`` defined earlier): class Booking(Predicate): date: datetime.date = field(DateField) - location: tuple[str, str] + location: Tuple[str, str] .. note:: @@ -685,7 +686,7 @@ above definition into something similar to the following (ignoring the class and class Booking(Predicate): date: datetime.date = field(DateField) - location: tuple[str, str] = field(SomeAnonymousName.Field) + location: Tuple[str, str] = field(SomeAnonymousName.Field) Here ``SomeAnonymousName`` has an empty name, so it will be treated as a tuple rather than a complex term with a function name. @@ -957,6 +958,68 @@ nesting. than lists; so in the above example we use ``(1,2,3)`` rather than ``[1,2,3]``. +Comparison Operators +-------------------- + +.. note: The following description of the comparison operator semantics applies to Clorm + version 1.6.0 and above. Earlier versions had a more ad-hoc approach to the comparison + operators. + + +The :class:`~clorm.Predicate` class overrides the standard Python comparison operators, such as +equality and the less than operators. To use the Clorm API effectively it is important to +understand how these operators are overriden. + +Predicate sub-classes should be understood as implementing a *view* over Clingo ``Symbol`` +objects. The view gives the user an easy and intuitive way to access the internal parameters of +the ``Symbol`` object. In the same way that the view does not alter the underlying ``Symbol`` +object, the view also does not alter the behavior of the comparison operators. Therefore the +comparison operators are based purely on the the comparison of the underlying ``Symbol`` +object. + +One important reason to use the underlying ``Symbol`` object for the comparison operators is +that clingo guarantees an ordering over all ``Symbol`` objects. This is in contrast to Python +types where not all operators are defined for objects of different types. For example, in +Python the non-equality operators are not defined for the ``int`` and ``str`` primitive types; +for example, ``"a" < 1`` will raise a ``TypeError``. However, in ASP programming it can be +natural to compare a constant (or string) and an integer term. + +This is particularly important for factbase querying, where sorting a set of facts will inherit +the ordering of the underlying ``Symbol`` object. + +A second consequence of this approach is that great care should be taken when using different +:class:`~clorm.Predicate` instances that map to the same underlying ``Symbol`` object. For example, + +.. code-block:: python + + from clingo import String, Function, Number + from clorm import Raw, Predicate, ConstantStr + + class Holds1(Predicate, name="holds"): + fluent: Raw + time: int + + class Holds2(Predicate, name="holds"): + fluent: int | ConstantStr + time: int + + h1 = Holds1(fluent=Raw(Number(100)), time=1) + h2 = Holds2(fluent=100, time=1) + + assert h1 == h2 + assert type(h1) != type(h2) + assert str(h1) == str(h2) + + assert len({h1, h2}) == 1 + +In this example the comparison ``h1 == h2`` holds even though they are of different types. A +consequence of this is ``h1`` and ``h2`` cannot be stored separate within the same set object, +so a set containing this two facts has the length of 1 rather than 2. This behaviour may seem +unintuitive at first glance. However consider that the main use of :class:`~clorm.Predicate` +instances is to read/write to/from the clingo solver, and within the solver these two instances +are indistinguishable. + + Old Syntax ---------- From 87f0e0abbb9bf80c4137c1699df4b367b3ec06f2 Mon Sep 17 00:00:00 2001 From: David Rajaratnam Date: Mon, 6 May 2024 21:53:04 +1000 Subject: [PATCH 4/9] Added example of FactBase.asp_str(sorted=True) --- tests/test_orm_factbase.py | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/tests/test_orm_factbase.py b/tests/test_orm_factbase.py index aa3491e..d9943a1 100644 --- a/tests/test_orm_factbase.py +++ b/tests/test_orm_factbase.py @@ -589,25 +589,16 @@ class Meta: # Test the asp output string with the sorted flag # -------------------------------------------------------------------------- def test_factbase_aspstr_sorted(self): - class A(Predicate): + class A(Predicate, name="bb"): a = IntegerField - class Meta: - name = "bb" - - class B(Predicate): + class B(Predicate, name="aa"): a = IntegerField - class Meta: - name = "aa" - - class C(Predicate): + class C(Predicate, name="aa"): a = IntegerField b = IntegerField - class Meta: - name = "aa" - def tostr(facts): return ".\n".join([str(f) for f in facts]) @@ -713,6 +704,20 @@ class Meta: self.assertIn(expected_sig_predC, result) self.assertIn(expected_sig_predA, result) + # -------------------------------------------------------------------------- + # Test the asp output string with sorting and incomparable terms + # -------------------------------------------------------------------------- + def test_factbase_aspstr_sorted_incomparable(self): + class A(Predicate): + x = field(SimpleField) + + fb = FactBase([A(1), A(2), A("b")]) + q = fb.query(A).ordered() + self.assertTrue(list(q.all()) == [A(1), A(2), A("b")]) + tmpstr1 = ".\n".join(f"{x}" for x in q.all()) + ".\n" + tmpstr2 = fb.asp_str(sorted=True) + self.assertTrue(tmpstr1 == tmpstr2) + # ------------------------------------------------------------------------------ # Test QueryAPI version 1 (called via FactBase.select() and FactBase.delete()) From 49d4d3a455e87e56051beecf5b74a3309c303379 Mon Sep 17 00:00:00 2001 From: David Rajaratnam Date: Mon, 6 May 2024 22:02:00 +1000 Subject: [PATCH 5/9] Black/isort formatting fixes --- clorm/orm/core.py | 8 +++++--- clorm/orm/query.py | 1 + clorm/orm/templating.py | 4 ++-- tests/support.py | 1 + tests/test_orm_core.py | 7 ------- tests/test_orm_factbase.py | 24 ++++++++++++++---------- 6 files changed, 23 insertions(+), 22 deletions(-) diff --git a/clorm/orm/core.py b/clorm/orm/core.py index 0746a56..fe9285b 100644 --- a/clorm/orm/core.py +++ b/clorm/orm/core.py @@ -1358,6 +1358,7 @@ class DateField(StringField): ``FactBase```. Defaults to ``False``. """ + def __init__(self, default: Any = MISSING, index: Any = MISSING) -> None: self._index = index if index is not MISSING else False @@ -1390,10 +1391,11 @@ def _process_cmplx_value(v): 'Invalid default value "{}" for {}'.format(default, type(self).__name__) ) else: + def _process_default(): return _process_value(default()) - self._default = (True, _process_default) + self._default = (True, _process_default) @staticmethod @abc.abstractmethod @@ -2800,7 +2802,7 @@ def _generate_dynamic_predicate_functions(class_name: str, namespace: Dict) -> N template = PREDICATE_TEMPLATE.format(pdefn=pdefn) predicate_functions = expand_template(template, **expansions) -# print(f"INIT {class_name}:\n\n{predicate_functions}\n\n") + # print(f"INIT {class_name}:\n\n{predicate_functions}\n\n") ldict: Dict[str, Any] = {} exec(predicate_functions, gdict, ldict) @@ -3179,6 +3181,7 @@ def _cltopy(v): # A Metaclass for the Predicate base class # ------------------------------------------------------------------------------ + @__dataclass_transform__(field_descriptors=(field,)) class _PredicateMeta(type): if TYPE_CHECKING: @@ -3460,7 +3463,6 @@ def __gt__(self, other): """Overloaded boolean operator.""" raise NotImplementedError("Predicate.__gt__() must be overriden") - @abc.abstractmethod def __hash__(self): """Overload the hash function.""" diff --git a/clorm/orm/query.py b/clorm/orm/query.py index 9ffdc19..2c2a030 100644 --- a/clorm/orm/query.py +++ b/clorm/orm/query.py @@ -536,6 +536,7 @@ def _try_to_convert(tup, possiblepath): return (args[0], _try_to_convert(args[1], args[0])) return args + class StandardComparator(Comparator): class Preference(enum.IntEnum): LOW = 0 diff --git a/clorm/orm/templating.py b/clorm/orm/templating.py index 287c8f0..1fd7f6f 100644 --- a/clorm/orm/templating.py +++ b/clorm/orm/templating.py @@ -44,9 +44,9 @@ def add_spaces(num, text): end = line.find(r"%}", start) if end == -1: raise ValueError("Bad template expansion in {line}") - keyword = line[start + 2:end] + keyword = line[start + 2 : end] text = add_spaces(start, kwargs[keyword]) - line = line[0:start] + text + line[end + 2:] + line = line[0:start] + text + line[end + 2 :] outlines.append(line) return "\n".join(outlines) diff --git a/tests/support.py b/tests/support.py index 2ce3be6..be7888c 100644 --- a/tests/support.py +++ b/tests/support.py @@ -25,6 +25,7 @@ def check_errmsg_contains(contmsg, ctx): # # ------------------------------------------------------------------------------ + def to_tuple(value): """Recursively convert a predicate/normal tuple into a Python tuple""" if isinstance(value, tuple) or (isinstance(value, Predicate) and value.meta.is_tuple): diff --git a/tests/test_orm_core.py b/tests/test_orm_core.py index 83520dd..c3cd8b9 100644 --- a/tests/test_orm_core.py +++ b/tests/test_orm_core.py @@ -1022,12 +1022,10 @@ class Blah(object): clresult = td.pytocl((1, "blah")) self.assertEqual(clresult, clob) - # -------------------------------------------------------------------------- # Test that we define the new comparison operators in the template # -------------------------------------------------------------------------- def test_predicate_comparison_operator_creation(self): - class P(Predicate, name="p"): a = IntegerField b = ConstantField @@ -1224,7 +1222,6 @@ class T(Predicate): self.assertTrue(p1.tuple_ != tuple1) self.assertFalse(tuple(p1.tuple_) != tuple1) - # -------------------------------------------------------------------------- # Testing comparison between tuple fields where the corresponding python tuples # can contain incomparible objects. @@ -1250,8 +1247,6 @@ class Q(Predicate): self.assertFalse(ptuple("a", "b") < qtuple(1, 2)) self.assertFalse(ptuple("a", "b") <= qtuple(1, 2)) - - # -------------------------------------------------------------------------- # Test predicates with default fields # -------------------------------------------------------------------------- @@ -2184,7 +2179,6 @@ class P(Predicate): self.assertTrue((neg_p2.raw < p2.raw) == (neg_p2 < p2)) self.assertTrue((p1.raw < p2.raw) == (p1 < p2)) - # -------------------------------------------------------------------------- # Test a simple predicate with a field that has a function default # -------------------------------------------------------------------------- @@ -2552,7 +2546,6 @@ class AComplex(ComplexTerm): self.assertTrue(f3 > f2) self.assertEqual(f3, f4) - # -------------------------------------------------------------------------- # Test unifying a symbol with a predicate # -------------------------------------------------------------------------- diff --git a/tests/test_orm_factbase.py b/tests/test_orm_factbase.py index d9943a1..ce24228 100644 --- a/tests/test_orm_factbase.py +++ b/tests/test_orm_factbase.py @@ -19,8 +19,8 @@ ConstantField, FactBase, IntegerField, - SimpleField, Predicate, + SimpleField, StringField, alias, asc, @@ -1708,6 +1708,7 @@ def test_api_bad_join_statement(self): # Test some special cases involving passing a Python tuple instead of using the clorm tuple. # ------------------------------------------------------------------------------------------ + class QueryWithTupleTestCase(unittest.TestCase): def setUp(self): class F(Predicate): @@ -1723,12 +1724,15 @@ class G(Predicate): self.factbase = FactBase( [ - F((1, "a"), "i"), F((2, "b"), "j"), F((3, "a"), "k"), - G(("a", "a"), "x"), G((2, "b"), "y"), G(("e", "e"), "z") + F((1, "a"), "i"), + F((2, "b"), "j"), + F((3, "a"), "k"), + G(("a", "a"), "x"), + G((2, "b"), "y"), + G(("e", "e"), "z"), ] ) - # -------------------------------------------------------------------------- # Some tuple predicate instance comparisons # -------------------------------------------------------------------------- @@ -1736,7 +1740,10 @@ def test_basic_clorm_tuple_in_comparison(self): G = self.G fb = self.factbase cltuple = G.atuple.meta.complex - expected = [G((2, "b"), "y"), G(("a", "a"), "x"), ] + expected = [ + G((2, "b"), "y"), + G(("a", "a"), "x"), + ] # NOTE: the version with python tuples fails to find the matching tuples because we can # no longer directly compare python tuples with clorm tuples. @@ -1750,7 +1757,6 @@ def test_basic_clorm_tuple_in_comparison(self): q2 = fb.query(G).where(in_(G.atuple, cltuples)).ordered() self.assertEqual(list(q2.all()), expected) - # -------------------------------------------------------------------------- # Complex query where the join is on a tuple object # -------------------------------------------------------------------------- @@ -1764,7 +1770,6 @@ def test_api_join_on_clorm_tuple(self): expected = [(G((2, "b"), "y"), F((2, "b"), "j"))] self.assertEqual(list(q.all()), expected) - # -------------------------------------------------------------------------- # Complex query with a where containing a tuple # -------------------------------------------------------------------------- @@ -1791,11 +1796,10 @@ def test_api_sorting_with_incomparable_elements(self): fb = self.factbase q = fb.query(G).order_by(G.atuple) - expected = [ - G((2, "b"), "y"), G(("a", "a"), "x"), G(("e", "e"), "z") - ] + expected = [G((2, "b"), "y"), G(("a", "a"), "x"), G(("e", "e"), "z")] self.assertEqual(list(q.all()), expected) + # ------------------------------------------------------------------------------ # Tests for additional V2 select join statements # ------------------------------------------------------------------------------ From ee746518d6ae1057f9a676946840b2f9c43dcf94 Mon Sep 17 00:00:00 2001 From: David Rajaratnam Date: Mon, 6 May 2024 22:16:00 +1000 Subject: [PATCH 6/9] Some reformatting and cleaning up --- clorm/_clingo.py | 4 +++- clorm/orm/core.py | 9 ++++++++- clorm/orm/noclingo.py | 1 + clorm/orm/query.py | 5 ++++- clorm/orm/templating.py | 1 + tests/support.py | 1 + tests/test_orm_atsyntax.py | 1 + tests/test_orm_factbase.py | 15 ++++++++++++--- tests/test_orm_symbols_facts.py | 1 + 9 files changed, 32 insertions(+), 6 deletions(-) diff --git a/clorm/_clingo.py b/clorm/_clingo.py index 108a94b..c9ade42 100644 --- a/clorm/_clingo.py +++ b/clorm/_clingo.py @@ -69,6 +69,7 @@ def _check_is_func(obj: Any, name: str) -> None: # Wrap clingo.Model and override some functions # ------------------------------------------------------------------------------ + # class Model(OModel, metaclass=WrapperMetaClass): class ModelOverride(object): """Provides access to a model during a solve call. @@ -236,6 +237,7 @@ class ClormSolveHandle(SolveHandleOverride, OSolveHandle): # Wrap clingo.Control and override some functions # ------------------------------------------------------------------------------ + # ------------------------------------------------------------------------------ # Helper functions to expand the assumptions list as part of a solve() call. The # assumptions list is a list of argument-boolean pairs where the argument can be @@ -255,7 +257,7 @@ def _add_fact(fact: Union[Predicate, Symbol], bval: bool) -> None: clingo_assump.append((raw, bool(bval))) try: - for (arg, bval) in assumptions: + for arg, bval in assumptions: if isinstance(arg, Predicate): _add_fact(arg, bval) elif isinstance(arg, Iterable): diff --git a/clorm/orm/core.py b/clorm/orm/core.py index fe9285b..7faada4 100644 --- a/clorm/orm/core.py +++ b/clorm/orm/core.py @@ -293,6 +293,7 @@ def __hash__(self): # conditions and other boolean conditions. # ------------------------------------------------------------------------------ + # comparator functions that always return true (or false). This is useful for # the cross product join operator that always returns true def trueall(x, y): @@ -1205,6 +1206,7 @@ def kwargs_check_keys(validkeys, inputkeys): # a field") between python and clingo. # ------------------------------------------------------------------------------ + # Create the pytocl and cltopy class member functions. If their inherit directly # from BaseField then just return the result of the function. If they inherit # from a sub-class of BaseField then call the parents conversion function first. @@ -1290,6 +1292,7 @@ def _raise_pytocl_nie(v): # and unifies, and the properties: default and has_default # ------------------------------------------------------------------------------ + # Mixin class to be able to use both MetaClasses class _AbstractBaseFieldMeta(abc.ABCMeta, _BaseFieldMeta): pass @@ -1817,6 +1820,7 @@ def _process_field_definition(field_defn: _FieldDefinition) -> Type[BaseField]: # restricts the allowable values. # ------------------------------------------------------------------------------ + # Support for refine_field def _refine_field_functor(subclass_name, field_class, valfunc): def _test_value(v): @@ -2563,11 +2567,11 @@ def _magic_name(name): PathIdentity = collections.namedtuple("PathIdentity", "predicate name") + # -------------------------------------------------------------------------- # One PredicateDefn object for each Predicate sub-class # -------------------------------------------------------------------------- class PredicateDefn(object): - """Encapsulates some meta-data for a Predicate definition. Each Predicate class will have a corresponding PredicateDefn object that specifies some @@ -2828,6 +2832,7 @@ def _set_fn(fname: str, docstring: str): # Metaclass constructor support functions to create the fields # ------------------------------------------------------------------------------ + # Generate a default predicate name from the Predicate class name. def _predicatedefn_default_predicate_name(class_name): @@ -3266,6 +3271,7 @@ def __iter__(self) -> Iterator[PredicatePath]: # underlying Symbol object. # ------------------------------------------------------------------------------ + # Mixin class to be able to use both MetaClasses class _AbstractPredicateMeta(abc.ABCMeta, _PredicateMeta): pass @@ -3546,6 +3552,7 @@ def simple_predicate( # Internal supporting functions # ------------------------------------------------------------------------------ + # ------------------------------------------------------------------------------ # Helper function to check if all the paths in a collection are root paths and # return path objects. diff --git a/clorm/orm/noclingo.py b/clorm/orm/noclingo.py index 87aba20..667fcd9 100644 --- a/clorm/orm/noclingo.py +++ b/clorm/orm/noclingo.py @@ -47,6 +47,7 @@ will raise an exception. """ + # -------------------------------------------------------------------------------- # -------------------------------------------------------------------------------- diff --git a/clorm/orm/query.py b/clorm/orm/query.py index 2c2a030..97471c9 100644 --- a/clorm/orm/query.py +++ b/clorm/orm/query.py @@ -57,12 +57,12 @@ # Defining and manipulating conditional elements # ------------------------------------------------------------------------------ + # ------------------------------------------------------------------------------ # Placeholder allows for variable substituion of a query. Placeholder is # an abstract class that exposes no API other than its existence. # ------------------------------------------------------------------------------ class Placeholder(abc.ABC): - r"""An abstract class for defining parameterised queries. Currently, Clorm supports 4 placeholders: ph1\_, ph2\_, ph3\_, ph4\_. These @@ -514,6 +514,7 @@ def membership_op_keyable(sc, indexes): # is not exposed outside clorm. # ------------------------------------------------------------------------------ + # Helper function to try to convert Python tuples into a matching clorm anon tuple. With this # we can pass a Python tuple to a query and not have to overload the Predicate comparison # operator and hash function to match the tuple. @@ -2233,6 +2234,7 @@ def num(sc): # describes the plan to execute a single link in a join. # ------------------------------------------------------------------------------ + # Check that the formula only refers to paths with the allowable roots def _check_roots(allowable_roots, formula): if not formula: @@ -3067,6 +3069,7 @@ def make_query_plan(indexed_paths, qspec): # generating an actual query. # ------------------------------------------------------------------------------ + # ------------------------------------------------------------------------------ # Creates a mechanism for sorting using the order_by statements within queries. # diff --git a/clorm/orm/templating.py b/clorm/orm/templating.py index 1fd7f6f..2400f89 100644 --- a/clorm/orm/templating.py +++ b/clorm/orm/templating.py @@ -21,6 +21,7 @@ def expand_template(template: str, **kwargs: str) -> str: line to preserve the correct indentation. """ + # Add spaces to each line of some multi-text input def add_spaces(num, text): space = " " * num diff --git a/tests/support.py b/tests/support.py index be7888c..66e81f2 100644 --- a/tests/support.py +++ b/tests/support.py @@ -4,6 +4,7 @@ # Support functions for the unit tests # ------------------------------------------------------------------------------ + # ------------------------------------------------------------------------------ # Helper function for helping to test for good error messages. # ------------------------------------------------------------------------------ diff --git a/tests/test_orm_atsyntax.py b/tests/test_orm_atsyntax.py index b23a968..203db82 100644 --- a/tests/test_orm_atsyntax.py +++ b/tests/test_orm_atsyntax.py @@ -426,6 +426,7 @@ def test_register(self): SF = StringField IF = IntegerField CF = ConstantField + # Functions to add to the context def add(a: IF, b: IF) -> IF: return a + b diff --git a/tests/test_orm_factbase.py b/tests/test_orm_factbase.py index ce24228..a94ecc6 100644 --- a/tests/test_orm_factbase.py +++ b/tests/test_orm_factbase.py @@ -589,16 +589,25 @@ class Meta: # Test the asp output string with the sorted flag # -------------------------------------------------------------------------- def test_factbase_aspstr_sorted(self): - class A(Predicate, name="bb"): + class A(Predicate): a = IntegerField - class B(Predicate, name="aa"): + class Meta: + name = "bb" + + class B(Predicate): a = IntegerField - class C(Predicate, name="aa"): + class Meta: + name = "aa" + + class C(Predicate): a = IntegerField b = IntegerField + class Meta: + name = "aa" + def tostr(facts): return ".\n".join([str(f) for f in facts]) diff --git a/tests/test_orm_symbols_facts.py b/tests/test_orm_symbols_facts.py index d121e89..6fca429 100644 --- a/tests/test_orm_symbols_facts.py +++ b/tests/test_orm_symbols_facts.py @@ -126,6 +126,7 @@ class Fun(ComplexTerm): anum = IntegerField() afun = Fun.Field() + # afun=ComplexField(Fun) class Meta: name = "afact" From 479775ea3227bbb56d6d8bf80f8151a9b7b15d99 Mon Sep 17 00:00:00 2001 From: David Rajaratnam Date: Mon, 6 May 2024 22:19:27 +1000 Subject: [PATCH 7/9] Update minimum Python version and update workflow --- .github/workflows/main.yml | 2 +- README.rst | 4 ++-- docs/index.rst | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index ab35bcc..e53e116 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -12,7 +12,7 @@ jobs: fail-fast: false matrix: os: ['ubuntu-latest', 'macos-latest', 'windows-latest'] - python-version: ['3.7', '3.8', '3.9', '3.10', '3.11', '3.12'] + python-version: ['3.8', '3.9', '3.10', '3.11', '3.12'] steps: - uses: actions/checkout@v2 diff --git a/README.rst b/README.rst index 3d6122e..439e6f1 100644 --- a/README.rst +++ b/README.rst @@ -20,12 +20,12 @@ that makes it easier to refactor the python code as the ASP program evolves. The documentation is available online `here `_. -Note: Clorm works with Python 3.7+ and Clingo 5.5+ +Note: Clorm works with Python 3.8+ and Clingo 5.6+ Installation ------------ -Clorm requires Python 3.7+ and Clingo 5.5+. It can be installed using either the +Clorm requires Python 3.8+ and Clingo 5.6+. It can be installed using either the `pip` or `conda` package managers. `pip` packages can be downloaded from PyPI: diff --git a/docs/index.rst b/docs/index.rst index 0978d7f..10e577a 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -22,7 +22,7 @@ generate problem instances and process the results dynamically. Clorm makes this integration cleaner, both in terms of code readability but also by making it easier to refactor the python code as the ASP program evolves. -- Works with Python 3.7+ and Clingo 5.5+ +- Works with Python 3.8+ and Clingo 5.6+ .. toctree:: From 3a6ca8c237401fc50ddfd5f2718c12719f75350e Mon Sep 17 00:00:00 2001 From: David Rajaratnam Date: Mon, 6 May 2024 22:20:12 +1000 Subject: [PATCH 8/9] Update version to 1.6.0 --- clorm/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clorm/__init__.py b/clorm/__init__.py index 7669106..84d6662 100644 --- a/clorm/__init__.py +++ b/clorm/__init__.py @@ -1,6 +1,6 @@ from .orm import * -__version__ = "1.5.1" +__version__ = "1.6.0" __author__ = "David Rajaratnam" __email__ = "daver@gemarex.com.au" __copyright__ = "Copyright (c) 2018 David Rajaratnam" From e648a6ddf1f1708420b1680f23c48bca24dbf1d2 Mon Sep 17 00:00:00 2001 From: David Rajaratnam Date: Mon, 6 May 2024 22:27:29 +1000 Subject: [PATCH 9/9] Remove abc.ABCMeta derivation of Predicate I think this is causing a problem with Python <= 3.10 when specifying Predicate with a "name" argument. --- clorm/orm/core.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/clorm/orm/core.py b/clorm/orm/core.py index 7faada4..8d23700 100644 --- a/clorm/orm/core.py +++ b/clorm/orm/core.py @@ -3272,12 +3272,7 @@ def __iter__(self) -> Iterator[PredicatePath]: # ------------------------------------------------------------------------------ -# Mixin class to be able to use both MetaClasses -class _AbstractPredicateMeta(abc.ABCMeta, _PredicateMeta): - pass - - -class Predicate(object, metaclass=_AbstractPredicateMeta): +class Predicate(object, metaclass=_PredicateMeta): """Abstract base class to encapsulate an ASP predicate or complex term. This is the heart of the ORM model for defining the mapping of a predicate @@ -3445,12 +3440,10 @@ def __neg__(self): # -------------------------------------------------------------------------- # Overloaded operators # -------------------------------------------------------------------------- - @abc.abstractmethod def __eq__(self, other): """Overloaded boolean operator.""" raise NotImplementedError("Predicate.__eq__() must be overriden") - @abc.abstractmethod def __lt__(self, other): """Overloaded boolean operator.""" raise NotImplementedError("Predicate.__lt__() must be overriden") @@ -3459,17 +3452,14 @@ def __le__(self, other): """Overloaded boolean operator.""" raise NotImplementedError("Predicate.__le__() must be overriden") - @abc.abstractmethod def __ge__(self, other): """Overloaded boolean operator.""" raise NotImplementedError("Predicate.__ge__() must be overriden") - @abc.abstractmethod def __gt__(self, other): """Overloaded boolean operator.""" raise NotImplementedError("Predicate.__gt__() must be overriden") - @abc.abstractmethod def __hash__(self): """Overload the hash function.""" raise NotImplementedError("Predicate.__hash__() must be overriden")