Skip to content
This repository

Error using idlsave #988

Closed
gsever opened this Issue November 10, 2011 · 8 comments

4 participants

Gökhan Sever Fernando Perez Min RK Thomas Kluyver
Gökhan Sever

Accessing s1['dn'] should work fine within IPython like in the regular Python shell.

I1 from scipy import io
I2 s1 = io.idl.readsav('test.sav')

I3 s1?
Type:       AttrDict
Base Class: <class 'scipy.io.idl.AttrDict'>
String Form:
{'dn': array([  1.02282184e+07,   1.05383408e+07,   1.08758739e+07,
           1.12449965e+07,   1. <...>  (('r', 'R'), '>f8'), (('v', 'V'), '>f8')]), 'tfit': array([  4.82394886e+02,   4.18176107e-01])}
Namespace:  Interactive
Length:     11
File:       /usr/lib64/python2.7/site-packages/scipy/io/idl.py
Definition: s1(self, name)


I4 s1['dn']
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
/home/gsever/Desktop/python-repo/ipython/IPython/core/prefilter.pyc in prefilter_lines(self, lines, continue_prompt)
    358                              for lnum, line in enumerate(llines) ])
    359         else:
--> 360             out = self.prefilter_line(llines[0], continue_prompt)
    361 
    362         return out

/home/gsever/Desktop/python-repo/ipython/IPython/core/prefilter.pyc in prefilter_line(self, line, continue_prompt)
    333             return normal_handler.handle(line_info)
    334 
--> 335         prefiltered = self.prefilter_line_info(line_info)
    336         # print "prefiltered line: %r" % prefiltered

    337         return prefiltered

/home/gsever/Desktop/python-repo/ipython/IPython/core/prefilter.pyc in prefilter_line_info(self, line_info)
    273         # print "prefilter_line_info: ", line_info

    274         handler = self.find_handler(line_info)
--> 275         return handler.handle(line_info)
    276 
    277     def find_handler(self, line_info):

/home/gsever/Desktop/python-repo/ipython/IPython/core/prefilter.pyc in handle(self, line_info)
    813 
    814         force_auto = isinstance(obj, IPyAutocall)
--> 815         auto_rewrite = getattr(obj, 'rewrite', True)
    816 
    817         if esc == ESC_QUOTE:

/usr/lib64/python2.7/site-packages/scipy/io/idl.pyc in __getitem__(self, name)
    657 
    658     def __getitem__(self, name):
--> 659         return super(AttrDict, self).__getitem__(name.lower())
    660 
    661     def __setitem__(self, key, value):

KeyError: 'rewrite'

Until this is fixed in IPy, the workaround is to use s1.get('dn')
Using Python 2.7, IPython 0.12.dev, scipy.0.11.0.dev-7e1bb79

Min RK
Owner

As @takluyver reported on-list, we should catch any error here, and not assume that objects are well behaved.

I would consider it a bug in AttrDict that missing attributes raises anything other than AttributeError, because it breaks extremely basic Python functionality.

Fernando Perez
Owner

I'm not sure we should catch all errors here: objects that raise WhoKnowsWhatError on attribute access are simply broken objects, and I'm worried that having blanket exception clause here can mask other problems in the future. So I'm inclined to say we leave things as they are, and that this error simply serves as a good warning system for people of bugs in their code :)

Min RK
Owner

From Python docs for hasattr():

...This is implemented by calling getattr(object, name) and seeing whether it raises an exception or not.

So hasattr itself checks for arbitrary errors, rather than just AttributeError.

In which case:

try:
   auto_rewrite = obj.rewrite
except Exception:
    auto_rewrite = True

is identical to

if hasattr(obj, 'rewrite'):
    auto_rewrite = obj.rewrite
else:
    auto_rewrite = True

Except the latter actually calls getattr(obj, 'rewrite') twice.

Thomas Kluyver
Collaborator

Raising other errors on attribute access is certainly awkward, but AFAIK it's not actually forbidden. Indeed, traits can do it - see #851. I think we should work gracefully with that sort of code, even if it's not ideal.

Fernando Perez
Owner

I reread the whole thing more carefully and I agree; the approach @minrk suggested with the try/except sounds good to me. No need for a PR, go ahead and push it if you want, unless you think there's more we should look into.

Thomas Kluyver
Collaborator

I'm going to hit the sack. @minrk, feel free to make this change, or if you've got better things to do, I'll do it tomorrow.

Fernando Perez
Owner

Done with tests added, I'll push in a sec.

Fernando Perez fperez closed this issue from a commit November 10, 2011
Fernando Perez Catch errors raised by user objects when accessing attributes.
When analyzing the line with prefilter, we look into whether objects
have a 'rewrite' attribute.  While this is off-spec, we've seen in the
wild objects that raise something other than AttributeError on
attribute access.  Now we catch all exceptions in this codepath.

Closes #988.
467b5b2
Fernando Perez fperez closed this in 467b5b2 November 10, 2011
Gökhan Sever

Thanks guys, the patch fixes the issue that I originally reported.

Fernando Perez fperez referenced this issue from a commit January 10, 2012
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.