Execfile

[jstenar]

Work related to #818

[fperez]

I thought py3 had removed execfile from the language... Did you actually test this on py3/non-windows? Does it work?

[jstenar]

Fernando Perez skrev 2011-10-18 21:55:

•        loc = loc if (loc is not None) else glob
  

•        scripttext = file(fname).read()
  

•        #compile converts unicode filename to str assuming
  

•        #ascii. Let's do the conversion before calling compile
  

•        if isinstance(fname, unicode):
  

•            filename = unicode_to_str(fname)
  

•        else:
  

•            filename = fname
  

•        exec compile(scripttext, filename, 'exec') in glob, loc
  

• else:

•    def execfile(fname, glob, loc=None):
  

•        if isinstance(fname, unicode):
  

•            filename = fname.encode(sys.getfilesystemencoding())
  

•        else:
  

•            filename = fname
  

•        **builtin**.execfile(filename, glob, loc)
  

I thought py3 had removed execfile from the language... Did you actually test this on py3/non-windows? Does it work?

Py3 version is created around line 71 (I have not touched that part).

[fperez]

On Tue, Oct 18, 2011 at 1:03 PM, Jörgen Stenarson
reply@reply.github.com
wrote:

Py3 version is created around line 71 (I have not touched that part).

Ah, OK; no problem then.

[fperez]

@jstenar, unfortunately when merged against current master, this PR doesn't pass the test suite. Running iptest -vvs IPython.core gives one failure:

======================================================================
ERROR: Test safe_execfile with non-ascii path
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/fperez/usr/lib/python2.7/site-packages/IPython/core/tests/test_interactiveshell.py", line 222, in test_1
    _ip.shell.safe_execfile(self.fname, raise_exceptions=True)
  File "/home/fperez/usr/lib/python2.7/site-packages/IPython/core/interactiveshell.py", line 2284, in safe_execfile
    py3compat.execfile(fname,*where)
  File "/home/fperez/usr/lib/python2.7/site-packages/IPython/utils/py3compat.py", line 172, in execfile
    __builtin__.execfile(filename, glob, loc)
TypeError: must be dict, not None

----------------------------------------------------------------------
Ran 353 tests in 1.176s

FAILED (KNOWNFAIL=2, errors=1)

Do you see it also on Windows? If not, we may need a bit more work to track it down, but if you see the error on Windows, just go ahead and fix it and we'll continue reviewing. Let us know if you need a hand!

[takluyver]

The builtin execfile doesn't use None as the defaults for the namespace arguments. I think we need to use the *where trick to pass it the correct number of arguments.

[fperez]

If you see the fix right away, feel free to just add one extra commit on top of @jstenar's work and then merge it with the fix... You can just follow steps 1 and 2 of the PR manual merge instructions (click on the 'i' at the left of the green merge bar), and then git reset --hard 9403023 to unwind the merge. Then apply the fix, commit and merge to master/push.

[fperez]

I do the above often with PRs where I see a tiny bit of manual cleanup at the end, which is quicker to do myself than to ask the submitter for another round of edits. I think that if the changes will teach the submitter something valuable of the development process (such as adding tests or docs that were missing) it's better to send it back. Similarly if the needed work is significant. But often there's one last bit of easy cleanup missing, and at that point it's just more work to go back and forth on the PR than to simply do it myself and push.

[takluyver]

OK, I'll get on it.

[takluyver]

Done. For reference, the extra change I made is here: 4adf668.

[fperez]

Thanks a ton, @takluyver! And also @jstenar, for the original work :)