On 02/12/2016 02:29 AM, immerrr again wrote:
On Thu, Feb 11, 2016 at 8:48 PM, Mike Bayer <[email protected]> wrote:
Good point, but this may not be an option. Currently we return NULLTYPE
for such ambigious comparisons but it's not known what side effects would
occur if we across the board assumed the other side should operate (plus the
other side needs to *never* return NotImplemented).
I don't see why does it need that. It can return NotImplemented, too.
In py3, if both sides of the binary operator return NotImplemented,
the operation itself is not implemented and a TypeError is raised. In
py2, for arithmetic operations the same happens, but for comparison
ops the result is that of "id(left) OP id(right)", which is good for
sorting collections of unorderable objects, but bad otherwise.
Given that the current default comparator returns a BinaryOperation
unconditionally, returning NotImplemented from it could indeed break
the existing code. But what if the comparator itself, before returning
the BinaryOperation as previously, would let the other side run the
corresponding reflected operation. If the result is NotImplemented,
then proceed with the usual behaviour, if not, return the result of
the reflected operation.
AFAICS, the only backward incompatible situation is when the other
side is silly enough to return some gibberish rather than
NotImplemented for unhandled operand types. Previously, it was not
even consulted, and now it would return gibberish instead of a
BinaryOperation. To me this sounds as an incorrect implementation of
Python spec.
I'm not really seeing at the moment how this is feasible considering
that one side or the other *has* to override the Python method and
return a BinaryExpression. Simply put, which side should win? If e.g.
I have an Integer type on one side, and a MagicInteger on the other,
what rule determines that Integer defers to MagicInteger ? What if
MagicInteger is compared against a SuperMagicInteger?
I think you're getting at this, that both sides somehow would want to
coordinate here. But I don't see why that has to happen all the way
down at the Python operator level as by the time we're making this
choice we're a few levels into a call stack already, and I can tell you
that the whole system of hooks already has a lot of paths that can
easily be tricked into going into endless loops if things arent just right.
What i had in mind is that the type / operator system would have some
other system of consulting the other side of the expression where it
gets a chance to veto the active-side's decision on what typing behavior
we want to coerce, likely within the _adapt_expression() method or close
to it.
A naive implementation of "return NotImplemented if we don't know what
to do with a type" looks like:
diff --git a/lib/sqlalchemy/sql/default_comparator.py
b/lib/sqlalchemy/sql/default_comparator.py
index 1bb1c34..c74e27f 100644
--- a/lib/sqlalchemy/sql/default_comparator.py
+++ b/lib/sqlalchemy/sql/default_comparator.py
@@ -82,6 +82,8 @@ def _binary_operate(expr, op, obj, reverse=False,
result_type=None,
if result_type is None:
op, result_type = left.comparator._adapt_expression(
op, right.comparator)
+ if result_type is NotImplemented:
+ return NotImplemented
return BinaryExpression(
left, right, op, type_=result_type, modifiers=kw)
diff --git a/lib/sqlalchemy/sql/sqltypes.py b/lib/sqlalchemy/sql/sqltypes.py
index 81630fe..9e0d01f 100644
--- a/lib/sqlalchemy/sql/sqltypes.py
+++ b/lib/sqlalchemy/sql/sqltypes.py
@@ -53,7 +53,7 @@ class _DateAffinity(object):
op, to_instance(
self.type._expression_adaptations.
get(op, self._blank_dict).
- get(othertype, NULLTYPE))
+ get(othertype, NotImplemented))
)
comparator_factory = Comparator
With just that, tons of basic tests fail. Feel free to work on this issue.
--
You received this message because you are subscribed to the Google Groups
"sqlalchemy" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at https://groups.google.com/group/sqlalchemy.
For more options, visit https://groups.google.com/d/optout.
