Setting __file__ to None breaks Mayavi import

[fperez]

As discussed at the bottom of #1831, after its merge it became impossible to import mayavi, with this traceback:

In [1]: import mayavi.mlab
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-1-c7e202e132f1> in <module>()
----> 1 import mayavi.mlab

/usr/lib/python2.7/dist-packages/mayavi/mlab.py in <module>()
     25 
     26 # Mayavi imports
---> 27 from mayavi.tools.camera import view, roll, yaw, pitch, move
     28 from mayavi.tools.figure import figure, clf, gcf, savefig, \
     29     draw, sync_camera, close, screenshot

/usr/lib/python2.7/dist-packages/mayavi/tools/camera.py in <module>()
     21 # We can't use gcf, as it creates a circular import in camera management
     22 # routines.
---> 23 from engine_manager import get_engine
     24 
     25 def world_to_display(x, y, z, figure=None):

/usr/lib/python2.7/dist-packages/mayavi/tools/engine_manager.py in <module>()
     10 
     11 # Local imports
---> 12 from mayavi.preferences.api import preference_manager
     13 from mayavi.core.registry import registry
     14 from mayavi.core.engine import Engine

/usr/lib/python2.7/dist-packages/mayavi/preferences/api.py in <module>()
      2 
      3 # The global PreferenceManager instance
----> 4 from preference_manager import preference_manager
      5 from bindings import set_scene_preferences, get_scene_preferences

/usr/lib/python2.7/dist-packages/mayavi/preferences/preference_manager.py in <module>()
    126 # A Global preference manager that all other modules can use.
    127 
--> 128 preference_manager = PreferenceManager()
    129 

/usr/lib/python2.7/dist-packages/mayavi/preferences/preference_manager.py in __init__(self, **traits)
     79 
     80         if 'preferences' not in traits:
---> 81             self._load_preferences()
     82 
     83     def _preferences_default(self):

/usr/lib/python2.7/dist-packages/mayavi/preferences/preference_manager.py in _load_preferences(self)
     99         """Load the default preferences."""
    100         # Save current application_home.
--> 101         app_home = ETSConfig.get_application_home()
    102         # Set it to where the mayavi preferences are temporarily.
    103         path = join(ETSConfig.get_application_data(), ID)

/usr/lib/python2.7/dist-packages/traits/etsconfig/etsconfig.pyc in get_application_home(self, create)
    127             self._application_home = path.join(
    128                                 self.get_application_data(create=create),
--> 129                                 self._get_application_dirname())
    130 
    131         return self._application_home

/usr/lib/python2.7/dist-packages/traits/etsconfig/etsconfig.pyc in _get_application_dirname(self)
    338         if main_mod is not None:
    339             if hasattr(main_mod, '__file__'):
--> 340                 main_mod_file = path.abspath(main_mod.__file__)
    341                 dirname = path.basename(path.dirname(main_mod_file))
    342 

/usr/lib/python2.7/posixpath.pyc in abspath(path)
    341 def abspath(path):
    342     """Return an absolute path."""
--> 343     if not isabs(path):
    344         if isinstance(path, unicode):
    345             cwd = os.getcwdu()

/usr/lib/python2.7/posixpath.pyc in isabs(s)
     51 def isabs(s):
     52     """Test whether a path is absolute"""
---> 53     return s.startswith('/')
     54 
     55 

AttributeError: 'NoneType' object has no attribute 'startswith'

It may be that there's an issue with mayavi (and if so, we can report it upstream), but breaking use of Mayavi is not acceptable. We may have to temporarily revert #1831 altogether, until we find a cleaner solution to #1814...

I'm not 100% sure on the semantics of __file__ so there may be a better solution that undoing #1831 completely.

[fperez]

This works in master as a temporary workaround for the mayavi import problem, further indicating that #1831 is really the culprit.

In [1]: del __file__

In [2]: import mayavi.mlab

# All OK, no traceback.

[rkern]

From the reference: "__file__ is the pathname of the file from which the module was loaded, if it was loaded from a file. The __file__ attribute is not present for C modules that are statically linked into the interpreter; for extension modules loaded dynamically from a shared library, it is the pathname of the shared library file."

I don't see any provision for it being None, only the actual file name or missing.

[fperez]

Thanks much, @rkern. Then it was indeed a mistake to merge #1831 in its original form, as the bug is squarely ours. Whew, glad we insisted on this going in post-0.13!

[bfroehle]

Looks like we need to clean up our handling of __file__ overall, in three different locations:

• IPython.core.interactiveshell.InteractiveShell.safe_execfile
• IPython.core.interactiveshell.InteractiveShell.safe_execfile_ipy
• Python.core.magics.execution.ExecutionMagics.run

I suggest that after running each of these __file__ should be restored to it's previous value or deleted.

[fperez]

On Fri, Aug 10, 2012 at 11:30 AM, Bradley M. Froehle <
notifications@github.com> wrote:

Looks like we need to clean up our handling of file overall, in three
different locations:

• IPython.core.interactiveshell.InteractiveShell.safe_execfile
• IPython.core.interactiveshell.InteractiveShell.safe_execfile_ipy
• Python.core.magics.execution.ExecutionMagics.run

I suggest that after running each of these file should be restored to
it's previous value or deleted.

Actually I would suggest something different: we should work identical to
how python itself works, which means:

No __file__ defined in the interactive prompt, nor in safe_execfile or
%run.

Python itself doesn't define __file__ either at the prompt or in
execfile, so neither should we. The case of running ipy files is
special, and we can consider settting and unsetting after execution
__file__ for those. But deviating from Python semantics here is a bad
idea, and we just got bitten by it with the mayavi problem.

[bfroehle]

Well, there are two schools of thought regarding %run... is it the same as python -i <filename> or execfile('<filename>')? Each of these has different semantics regarding whether or not __file__ is defined:

$ cat tst.py 
print __file__

$ python -i tst.py 
tst.py
>>> __file__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name '__file__' is not defined

$ python
>>> execfile('tst.py')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "tst.py", line 1, in <module>
    print __file__
NameError: name '__file__' is not defined
>>> __file__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name '__file__' is not defined

Note that in no case does __file__ leak back into the global scope.

[bfroehle]

I should mention that I think we got bitten not because we deviated from Python semantics but because we failed to properly restore the previous context.

Consider our current code:

save_fname = namespace.get('__file__', None)
try:
    ...
finally:
    namespace['__file__'] = save_fname

As compared to what I would consider more correct code.

save_fname = namespace.get('__file__', None)
try:
    ...
finally:
    if save_fname is not None:
        namespace['__file__'] = save_fname
    else:
        # None is never a valid __file__
        del namespace['__file__']

[fperez]

On Fri, Aug 10, 2012 at 12:40 PM, Bradley M. Froehle <
notifications@github.com> wrote:

Well, there are two schools of thought regarding %run... is it the same
as python -i or execfile('')? Each of these has
different semantics regarding whether or not file is defined:

Right, good point %run X is really more of a python -i X than
execfile('X'), especially given the trouble we go to with handling
sys.argv as if it was being run at a system prompt.

But indeed, it should never leak back out post-execution.

Cheers,

f

[bfroehle]

At this point I'm in favor of a wholesale revert of #1831 -- the original patch which added this __file__ injection. It's caused a lot of grief and is clearly implemented incorrectly.

In addition to not fully restoring the context, in safe_execfile it sets __file__ in the wrong dictionary! Instead of in *where, the tuple that gets passed to execfile it sets the variable in self.user_ns. This only happens to work because we generally call safe_execfile(..., self.user_ns).

[fperez]

@bfroehle, +1 on that. Go for it, if you have the bandwidth right now.

[minrk]

closed by #2432