IPython shell swallows exceptions in certain circumstances

[sccolbert]

A vanilla Python shell session:

Chris-Colberts-MacBook-Pro:enaml chris$ python
Enthought Python Distribution -- www.enthought.com
Version: 7.1-2 (32-bit)

Python 2.7.2 |EPD 7.1-2 (32-bit)| (default, Jul  3 2011, 15:40:35) 
[GCC 4.0.1 (Apple Inc. build 5493)] on darwin
Type "packages", "demo" or "enthought" for more information.
>>> from traits.api import *
>>> class Foo(HasTraits):
...     a = Int
... 
>>> def default(self):
...     self.remove_trait('a')
...     setattr(self, 'a', 42.0)
...     return 42.0
... 
>>> c = Any().as_ctrait()
>>> c.default_value(8, default)
>>> f = Foo()
>>> f.add_trait('a', c)
>>> f.a
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 3, in default
  File "/Library/Frameworks/Python.framework/Versions/7.1/lib/python2.7/site-packages/traits/trait_handlers.py", line 168, in error
    value )
traits.trait_errors.TraitError: The 'a' trait of a Foo instance must be an integer, but a value of 42.0 <type 'float'> was specified.
>>> f.a
0

The equivalent IPython session:

Python 2.7.2 |EPD 7.1-2 (32-bit)| (default, Jul  3 2011, 15:40:35) 
Type "copyright", "credits" or "license" for more information.

IPython 0.11 -- An enhanced Interactive Python.
?         -> Introduction and overview of IPython's features.
%quickref -> Quick reference.
help      -> Python's own help system.
object?   -> Details about 'object', use 'object??' for extra details.

In [1]: from traits.api import *

In [2]: def default(self):
   ...:     self.remove_trait('a')
   ...:     setattr(self, 'a', 42.0)
   ...:     return 42.0
   ...: 

In [3]: class Foo(HasTraits):
   ...:     a = Int
   ...:     

In [4]: c = Any().as_ctrait()

In [5]: c.default_value(8, default)

In [6]: f = Foo()

In [7]: f.add_trait('a', c)

In [8]: f.a
Out[8]: 0

The IPython shell swallows the exception raised by the default function. It appears that IPython is doing subsequent getattr(obj, name) calls (which succeed in this case) after the exception is raised.

[fperez]

Ouch, very true! Here's the relevant code in standalone format so it's easier to put into a file for running:

"""
https://github.com/ipython/ipython/issues/851
"""
from traits.api import *

class Foo(HasTraits):
    a = Int

def default(self):
    self.remove_trait('a')
    setattr(self, 'a', 42.0)
    return 42.0

c = Any().as_ctrait()
c.default_value(8, default)

f = Foo()
f.add_trait('a', c)

To test, use:

In [1]: run bug851.py

In [2]: f.a
Out[2]: 0

In contrast, the expected behavior is:

>>> execfile('bug851.py')
>>> f.a
Traceback (most recent call last):
  File "", line 1, in 
  File "bug851.py", line 11, in default
    setattr(self, 'a', 42.0)
  File "/usr/lib/pymodules/python2.6/traits/trait_handlers.py", line 168, in error
    value )
traits.trait_errors.TraitError: The 'a' trait of a Foo instance must be an integer, but a value of 42.0  was specified.

Confirming this, though it may take some digging to figure out where we are being over-aggressive. But this is pretty serious. Oddly enough, it's been there since 0.10.x (which means probably for a really long time), and nobody had ever reported the problem. But no matter, we need to fix it.

[sccolbert]

Leave it to me to... http://imgur.com/ZLjjL

On Mon, Oct 10, 2011 at 3:29 PM, Fernando Perez <
reply@reply.github.com>wrote:

Ouch, very true! Here's the relevant code in standalone format so it's
easier to put into a file for running:

"""
https://github.com/ipython/ipython/issues/851
"""
from traits.api import *

class Foo(HasTraits):
   a = Int

def default(self):
   self.remove_trait('a')
   setattr(self, 'a', 42.0)
   return 42.0

c = Any().as_ctrait()
c.default_value(8, default)

f = Foo()
f.add_trait('a', c)

To test, use:

In [1]: run bug851.py

In [2]: f.a
Out[2]: 0

In contrast, the expected behavior is:

>>> execfile('bug851.py')
>>> f.a
Traceback (most recent call last):
 File "<stdin>", line 1, in <module>
 File "bug851.py", line 11, in default
   setattr(self, 'a', 42.0)
 File "/usr/lib/pymodules/python2.6/traits/trait_handlers.py", line 168, in
error
   value )
traits.trait_errors.TraitError: The 'a' trait of a Foo instance must be an
integer, but a value of 42.0 <type 'float'> was specified.

Confirming this, though it may take some digging to figure out where we are
being over-aggressive. But this is pretty serious. Oddly enough, it's been
there since 0.10.x (which means probably for a really long time), and nobody
had ever reported the problem. But no matter, we need to fix it.

Reply to this email directly or view it on GitHub:
#851 (comment)

[fperez]

On Mon, Oct 10, 2011 at 1:37 PM, sccolbert
reply@reply.github.com
wrote:

Leave it to me to... http://imgur.com/ZLjjL

Yeah, collect your achievement badge on your way out, kiddo!

;)

[takluyver]

I notice from the initial vanilla Python session that it doesn't throw an error the second time you do f.a. It seems likely that our machinery does the first attribute access somewhere before running the user code, and catches the exception. Tab completion is certainly allowed to try attribute access, and I wouldn't be surprised if something in the prefilter code does too. Attribute access changing state is nasty and unusual, so I don't think we should worry too much about it.

If you do want to see where it's failing, you might want to see whether you can override the error method to raise BaseException instead of TraitError - if we've used the standard except Exception paradigm, BaseException won't be silenced. (As an aside, this is the reason to use that rather than catching everything).

[sccolbert]

On Thu, Nov 10, 2011 at 1:31 PM, Thomas <
reply@reply.github.com>wrote:

I notice from the initial vanilla Python session that it doesn't throw an
error the second time you do f.a. It seems likely that our machinery does
the first attribute access somewhere before running the user code, and
catches the exception. Tab completion is certainly allowed to try attribute
access, and I wouldn't be surprised if something in the prefilter code does
too. Attribute access changing state is nasty and unusual, so I don't think
we should worry too much about it.

This could rear up and bite again for lots of things: properties,
descriptors, getattribute overrides, etc... There are lots cases in
lazy compute patterns where an attribute access may or may not change some
state, but could nevertheless raise a subclass of Exception.

[takluyver]

There are plenty of ways to do it, but I think the cases where it's a good
idea are rare.

We make various assumptions that certain actions won't have side effects,
when that's simply a convention, not a technical limitation. Similarly, tab
completing import statements is allowed to load modules, even though that
could have side effects. Perhaps we should have a 'paranoia mode' where we
refrain from automatically doing anything that could have side effects, but
I can't see it becoming the default.

[sccolbert]

On Thu, Nov 10, 2011 at 4:39 PM, Thomas <
reply@reply.github.com>wrote:

There are plenty of ways to do it, but I think the cases where it's a good
idea are rare.

We make various assumptions that certain actions won't have side effects,
when that's simply a convention, not a technical limitation. Similarly, tab
completing import statements is allowed to load modules, even though that
could have side effects. Perhaps we should have a 'paranoia mode' where we
refrain from automatically doing anything that could have side effects, but
I can't see it becoming the default.

I'm not advocating that you shouldn't do it. I'm advocating simply that it
shouldn't swallow the exception without re-raising it after doing whatever
post-introspection processing needs to be done. In this example, we never
requested auto-complete via Tab press, so the expected behavior would be
the same as if PyObject_GetAttr were called.

[takluyver]

Perhaps, but:

• The user would see exceptions in addition to what happened in actually
  executing their code, which is confusing. In this example, you would see
  the exception and the returned value from the execution. If the attribute
  access consistently raised an error (e.g. for a nonexistent attribute -
  much more common than this), the user would see that error twice.
• The exception here is not really the issue: it's the fact that state is
  changed so that the same attribute access the next time behaves
  differently. You would see something similar if you overrode __getattr__
  to increment a counter - it would be incremented twice from a single
  execution (and possibly more times from tab completion). We assume that
  attribute access doesn't have side effects, which is probably correct 99.9%
  of the time, and very convenient.

[sccolbert]

On Thu, Nov 10, 2011 at 6:13 PM, Thomas <
reply@reply.github.com>wrote:

Perhaps, but:

• The user would see exceptions in addition to what happened in actually
  executing their code, which is confusing. In this example, you would see
  the exception and the returned value from the execution. If the attribute
  access consistently raised an error (e.g. for a nonexistent attribute -
  much more common than this), the user would see that error twice.
• The exception here is not really the issue: it's the fact that state is
  changed so that the same attribute access the next time behaves
  differently. You would see something similar if you overrode __getattr__
  to increment a counter - it would be incremented twice from a single
  execution (and possibly more times from tab completion). We assume that
  attribute access doesn't have side effects, which is probably correct 99.9%
  of the time, and very convenient.

That's valid, but it somehow feels less wrong that swallowing exceptions.
What if the instrospection code only cause AttributeError instead of
Exception?

[takluyver]

The distinction is that its an exception appearing in IPython's internal
machinery, not from running user code. We mostly try to hide those.

If we simply limited ourself to catching AttributeError, other errors
appearing on attribute access would probably blow things up in
unpredictable ways. As it happens, we've just been discussing something
very similar on issue #988, and come to the conclusion that we should catch
all exceptions in that case.

[fperez]

@sccolbert, note that you can get the behavior you're asking for, if you disable completely autocalling. Here's an example in an IPython session:

In [1]: >>> from traits.api import *

In [2]: >>> class Foo(HasTraits):
   ...:     ...     a = Int
   ...:     ... 

In [3]: >>> def default(self):
   ...:     ...     self.remove_trait('a')
   ...:     ...     setattr(self, 'a', 42.0)
   ...:     ...     return 42.0
   ...: ... 

In [4]: >>> c = Any().as_ctrait()

In [5]: >>> c.default_value(8, default)

In [6]: >>> f = Foo()

In [7]: >>> f.add_trait('a', c)

In [8]: %autocall 0
Automatic calling is: OFF

In [9]: f.a
---------------------------------------------------------------------------
TraitError                                Traceback (most recent call last)
/home/fperez/tmp/junk/<ipython-input-9-adb09fc7d97f> in <module>()
----> 1 f.a

/home/fperez/tmp/junk/<ipython-input-3-47c366e94885> in default(self)
      1 def default(self):
      2     self.remove_trait('a')
----> 3     setattr(self, 'a', 42.0)
      4     return 42.0
      5 

/usr/lib/python2.6/dist-packages/traits/trait_handlers.py in error(self, object, name, value)
    166         """
    167         raise TraitError( object, name, self.full_info( object, name, value ),
--> 168                           value )
    169 
    170     def arg_error ( self, method, arg_num, object, name, value ):

TraitError: The 'a' trait of a Foo instance must be an integer, but a value of 42.0 <type 'float'> was specified.

So the take-home message is: if you have code that relies on delicate attribute handling semantics, you may want to simply give up autocall altogether. You can permanently disable it in your config file, just look for 'autocall' in your default profile (create one if you don't have it with ipython profile create.

Note that we're discussing whether to change this default in the future on the dev list.

But in the meantime I'm closing this, since there's really nothing we can 'fix': the fix is to turn autocall off, and we'll decide whether to do it system-wide, but users can always do it now either for just one session with %autocall 0 or permanently in their profile.