[PEAK] IsSubclass test reordering [WAS: Patch to clear RuleSet]

[Phillip J. Eby]

At 04:54 PM 7/16/2008 +0200, Alberto Valverde wrote:
> > At 10:25 PM 7/15/2008 +0200, Alberto Valverde wrote:
> >>Phillip J. Eby wrote:
> > The bigger issue is what types of tests should be allowed to go
> > first.  Currently, anything but issubclass() that's a test on a plain
> > argument is allowed to run in any order, but I can think of error
> > scenarios for everything except "is", especially if you consider that
> > one argument's contents might dictate the contents of another.
>
>Eeeek, one argument modifying the contents of another in an expression
>that will be evaluated when doing rule dispatch sounds like begging for
>trouble anyway. peak.rules should not worry about this use-case at all
>IMHO but consumers of the library should make sure this can never happen.

That's not what I mean.  Consider a function whose arguments are 
interdependent: if I pass "1" in argument 1, then argument 2 is 
required to be a FooBar that can thus be compared against another 
FooBar.  In that case, a rule that tests argument 1 for equality to 1 
followed by some other test that assumes argument 2 is a FooBar will 
fail.  This works just fine for attributes or other expressions 
operating on argument 2; it's only if argument 2 is then directly 
compared to something else that there could be a problem.


>BTW, any tips on how to trace which functions are being called under what
>rules? I hacked something for RuleDispatch some time ago [1] (the
>_make_func_logger) but felt kind of dirty. Is there any way to implement
>similar functionality, more elegantly, in peak.rules? I've though of an
>around() method that logged every next_method's func_name or something but
>then I realized that will be called just once for each call to the "base"
>gf, not on every "around", "before", "after" and chained "when"s that
>might be executed for a single call to the "base" gf (I'm sure there's a
>formal name for "base gf"... but unknown to me, sorry). Please note that
>this is not a feature request, I know you're a busy man, but a request for
>a small bump in the right direction so I can implement it myself.

I believe the repr() of Method objects is sufficient to dump out what 
you're looking for.  So you could just make an Around-like method 
with top precedence, that logs the repr of its tail.


> > The flip side is to just require tests to always occur in the order
> > of appearance; this is more predictable and "safer", but could carry
> > a performance cost if the order used in the rules as written
> > conflicts with the optimum order from a selectivity point of view.
>
>I'm fine with letting peak.rules optimize then order of the tests for me,
>in fact, I'd like it to do it as it does now. Just adding some internal
>rules to handle these edge cases like you did is enough I think (since
>using issubclass to dispatch on the types of classes, although less common
>than the type of instances, is a reasonably frequent use-case IMHO).
>
>Maybe this is crazy but... couldn't peak.rules implement an IsSubclass
>check that is smarter than python's so it doesn't blow up on non-classes
>but return a safer False like isinstance does instead?

Probably, but that's a feature request, not a bug fix.  :)  You can, 
btw, now define a meta_function on inspect.isclass to allow it to be 
translated into a faster test that will interoperate correctly with 
non-class rules, e.g.:

from peak.rules.predicates import meta_function, IsInstance
from peak.rules.criteria import Disjunction, Class, Test

class_types = Disjunction([Class(type), Class(ClassType)])

@meta_function(inspect.isclass)
def compile_isclass(expr):
     return Test(IsInstance(expr), class_types)

The above translates any staticaly-detectable call to 'isclass(expr)' 
into an 'isinstance(expr, (type, ClassType))' at compile time, which 
means that 'isclass()' doesn't get called and the condition is tested 
simultaneously with any other isinstance tests on expr; it also means 
that there's less ambiguity between isclass-based rules and 
non-isclass based rules.