epiphany

[Steven D'Aprano]

On Wed, 24 Apr 2013 19:50:33 -0400, Roy Smith wrote:

> I discovered something really neat today.
> 
> We've got a system with a bunch of rules.  Each rule is a method which
> returns True or False.  At some point, we need to know if all the rules
> are True.  Complicating things, not all the rules are implemented. Those
> that are not implemented raise NotImplementedError.


NotImplementedError is intended to be raised by abstract base classes to 
indicate a method that must be overridden. I also use it as a place-
holder for functions or methods I haven't actually written yet. I'm not 
sure what semantics you're giving NotImplementedError in your code, but I 
wonder whether a neater solution might be to just use rule = None for 
unimplemented rules, rather than:

def unimplemented():
    raise NotImplementedError

rule = unimplemented

Then your logic for seeing if all rules return true would become:

all(r() for r in rules if r is not None)

and for seeing if all rules return true or are unimplemented:

all(r is None or r() for r in rules)



> We used to have some ugly logic which kept track of which rules were
> active and only evaluated those.

I don't see why you would need anything like that. Reading further on, I 
see that you are counting unimplemented rules as true, for some reason 
which I don't understand. (Knowing nothing of your use-case, I would have 
expected intuitively that unimplemented rules count as not true.) A 
simple helper function will do the job:


def eval(rule):
    try:
        return rule()
    except NotImplementedError:
        return True

everything_is_true = all(eval(r) for r in rules)



No need for complicated ugly logic keeping track of what rules are 
implemented. But if you're worried about the cost of catching those 
exceptions (you've profiled your code, right?) then that's easy with a 
decorator:


def not_implemented(func):
    @functools.wraps(func)
    def inner(*args, **kw):
        raise NotImplementedError
    inner.ni = True
    return inner


# Decorate only the rules you want to be unimplemented.

@not_implemented
def my_rule():
    pass


everything_is_true = all(r() for r in rules if not hasattr(r, 'ni'))



Note that if you could reverse the logic so that unimplemented rules 
count as not true, this will also work:

try:
    everything_is_true = all(r() for r in rules)
except NotImplementedError:
    everything_is_true = False



> So, here's the neat thing.  It turns out that bool(NotImplemented)
> returns True.  By changing the unimplemented rules from raising
> NotImplementedError to returning NotImplemented, the whole thing
> becomes:
> 
>     return all(r() for r in rules)

Objects are supposed to return NotImplemented from special dunder methods 
like __add__, __lt__, etc. to say "I don't know how to implement this 
method for the given argument". Python will then try calling the other 
object's special method. If both objects return NotImplemented, Python 
falls back on whatever default behaviour is appropriate.

So, knowing nothing of your application, I fear that this is an abuse of 
NotImplemented's semantics. If a rule returns NotImplemented, I would 
expect your application to fall back on a different rule. If that's not 
the case, you're using it in a non-standard way that will cause confusion 
for those with expectations of what NotImplemented means.



-- 
Steven