EAFP gone wrong

[Arnaud Delobelle]

Hi all,

Hi have a set of classes that represent mathematical objects which can
be represented as a string using a 'latex' method (after Knuth's famous
typesetting system). As I want to be able to typeset some builtin types as
well, I have a generic function, latex(), as follows:

def latex(val):
try:
return val.latex()
except AttributeError:
if isinstance(val, (tuple, list)):
return ", ".join(map(latex, val))
elif isinstance(val, dict):
return ", ".join(
"%s=%s" % (latex(k), latex(v))
for k, v in sorted(val.iteritems())
)
else:
return str(val)

It's EAFP and I have used this for a while with no problem. Recently I
added a new class for 'n choose r' objects, as follows:

class Choose(Expression):
def __init__(self, n, r):
self.subexprs = n, r
self.n = n
self.r = r
def calc(self, ns=None, calc=calc):
return choose(calc(self.n, ns), calc(self.r, ns))
def latex(self):
return "{%s \\choose %s}" % (latex(self.n), latex(self.k))

When I create a Choose object and try to get its latex representation,
this happens:
'<qmm.maths.expressions.Choose object at 0x17c92d0>'

This puzzled me for a bit: why is it not trying to use the latex()
method of the Choose object? I read and reread the definition of the
latex() method for a while until I found that there was a typo. Where
it says:

latex(self.k)

it should say:

latex(self.r)

Thus it triggers an AttributeError, which is exactly the kind of
exception that I am catching in the latex() function after trying
val.latex(). (Of course I could have caught this by calling c.latex()
directly but it's such a short method definition that I couldn't imagine
missing the typo!).

This means that EAFP made me hide a typo which would have been obviously
detected had I LBYLed, i.e. instead of

try:
return val.latex()
except AttributeError:
...

do

if hasattr(val, 'latex'):
return val.latex()
else:
...


So was it wrong to say it's EAFP in this case? Should I have known to
LBYL from the start? How do you decide which one to use? Up to now, I
thought it was more or less a matter of taste but now this makes me
think that at least LBYL is better than catching AttributeError.

Thanks for any guidance.

 

[Malte Helmert]

This means that EAFP made me hide a typo which would have been obviously
detected had I LBYLed, i.e. instead of

try:
return val.latex()
except AttributeError:
...

do

if hasattr(val, 'latex'):
return val.latex()
else:
...


So was it wrong to say it's EAFP in this case?

I would say that it's not really the EAFP concept that is problematic
here, but rather that the try block encompasses more code than it
should. Generally try blocks should be as narrow as possible, i.e., they
should contain only the part where you really want to catch a potential
failure.

"return val.latex()" does two separate things that might fail: the
lookup of val.latex, and the actual method call. If I understood you
correctly, you only want to catch the AttributeError in the "val.latex"
lookup here, and hence I'd say the "correct" application of EAFP here
would be something like:

try:
foo = val.latex
except AttributeError:
...
else:
return foo()

Whether that's any better than LBYL in this particular case is of course
debatable -- one nice thing compared to your LBYL version is that it
doesn't look up val.latex twice upon success. But you could also get
that via the LBYLish:

latex_method = getattr(val, "latex")
if latex_method:
return latex_method()
....

Malte

 

[Malte Helmert]

def latex(val):
def make_result_in_the_absence_of_a_latex_method():
result = transmogrify(val)
return result

try:
typeset_func = val.latex
except AttributeError:
typeset_func = make_result_in_the_absence_of_a_latex_method

result = typeset_func()
return result

In this particular case, where in the case of an AttributeError you want
to use a fallback callable with the same signature as the bound method
you get in case of success, I'd say getattr with a default is the nicest
approach:

def latex(val):
def make_result_in_the_absence_of_a_latex_method():
result = transmogrify(val)
return result
return getattr(val, "latex",
make_result_in_the_absence_of_a_latex_method)()


Doesn't work as nicely if you don't have
make_result_in_the_absence_of_a_latex_method's functionality bundled
into a suitable function already, though.

Malte

 

[Matthew Barnett]

Hi all,

Hi have a set of classes that represent mathematical objects which can
be represented as a string using a 'latex' method (after Knuth's famous
typesetting system). As I want to be able to typeset some builtin types as
well, I have a generic function, latex(), as follows:

def latex(val):
try:
return val.latex()
except AttributeError:
if isinstance(val, (tuple, list)):
return ", ".join(map(latex, val))
elif isinstance(val, dict):
return ", ".join(
"%s=%s" % (latex(k), latex(v))
for k, v in sorted(val.iteritems())
)
else:
return str(val)

It's EAFP and I have used this for a while with no problem. Recently I
added a new class for 'n choose r' objects, as follows:

class Choose(Expression):
def __init__(self, n, r):
self.subexprs = n, r
self.n = n
self.r = r
def calc(self, ns=None, calc=calc):
return choose(calc(self.n, ns), calc(self.r, ns))
def latex(self):
return "{%s \\choose %s}" % (latex(self.n), latex(self.k))

When I create a Choose object and try to get its latex representation,
this happens:

'<qmm.maths.expressions.Choose object at 0x17c92d0>'

This puzzled me for a bit: why is it not trying to use the latex()
method of the Choose object? I read and reread the definition of the
latex() method for a while until I found that there was a typo. Where
it says:

latex(self.k)

it should say:

latex(self.r)

Thus it triggers an AttributeError, which is exactly the kind of
exception that I am catching in the latex() function after trying
val.latex(). (Of course I could have caught this by calling c.latex()
directly but it's such a short method definition that I couldn't imagine
missing the typo!).

This means that EAFP made me hide a typo which would have been obviously
detected had I LBYLed, i.e. instead of

try:
return val.latex()
except AttributeError:
...

do

if hasattr(val, 'latex'):
return val.latex()
else:
...


So was it wrong to say it's EAFP in this case? Should I have known to
LBYL from the start? How do you decide which one to use? Up to now, I
thought it was more or less a matter of taste but now this makes me
think that at least LBYL is better than catching AttributeError.

Thanks for any guidance.

In addition to the other replies, you should've tested the Choose class.

 

[Arnaud Delobelle]

I would say that it's not really the EAFP concept that is problematic
here, but rather that the try block encompasses more code than it
should. Generally try blocks should be as narrow as possible, i.e., they
should contain only the part where you really want to catch a potential
failure.

"return val.latex()" does two separate things that might fail: the
lookup of val.latex, and the actual method call. If I understood you
correctly, you only want to catch the AttributeError in the "val.latex"
lookup here, and hence I'd say the "correct" application of EAFP here
would be something like:

try:
foo = val.latex
except AttributeError:
...
else:
return foo()

Whether that's any better than LBYL in this particular case is of course
debatable -- one nice thing compared to your LBYL version is that it
doesn't look up val.latex twice upon success. But you could also get
that via the LBYLish:

latex_method = getattr(val, "latex")
if latex_method:
return latex_method()

Ben, Malte,

Thanks for your informative replies. I can now see where my blind spot
was and that I never really used EAFP properly before. I think I will
go with one of the two solutions above (probably the second) as I feel
it's a bit heavy handed to create a closure and perhaps call it every
time the latex() function is called.

Matthew,

You are right. Believe me I am fully aware of this. To my shame I have
many thousands lines of Python without any tests in this project and I
don't even know where to start...

Thanks again,