Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Bug fix for approval #1452

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
4 participants

This is a fix for a simple bug that was done at the Python Sheffield user group. The bug was that running a file in debug mode in Python 3 caused a NameError to be thrown for execfile.

@takluyver takluyver and 1 other commented on an outdated diff Feb 29, 2012

IPython/core/magic.py
@@ -1740,7 +1740,9 @@ def magic_run(self, parameter_s ='', runner=None,
main_mod = self.shell.new_main_mod()
prog_ns = main_mod.__dict__
prog_ns['__name__'] = name
-
+
+ prog_ns['execfile'] = 'py3compat.execfile'
@takluyver

takluyver Feb 29, 2012

Owner

This needs to be the function itself, not the name (i.e. lose the quotation marks)

@dchetwynd

dchetwynd Feb 29, 2012

Thanks Thomas,

I'll make the change tonight and submit another pull request.

Daley

On Tue, Feb 28, 2012, at 04:01 PM, Thomas wrote:

@@ -1740,7 +1740,9 @@ def magic_run(self, parameter_s ='', runner=None,
main_mod = self.shell.new_main_mod()
prog_ns = main_mod.dict

prog_ns['name'] = name

  •    prog_ns['execfile'] = 'py3compat.execfile'
    

This needs to be the function itself, not the name (i.e. lose the
quotation marks)


Reply to this email directly or view it on GitHub:
https://github.com/ipython/ipython/pull/1452/files#r499306

http://www.fastmail.fm - Does exactly what it says on the tin

@takluyver

takluyver Feb 29, 2012

Owner

You don't need to submit a new PR - if you commit the change on the same branch and push it, the pull request will automatically get updated ;-)

dchetwynd added some commits Feb 29, 2012

@takluyver takluyver commented on the diff Mar 1, 2012

IPython/core/magic.py
@@ -1871,6 +1873,9 @@ def magic_run(self, parameter_s ='', runner=None,
# exit.
self.shell.user_ns['__builtins__'] = builtin_mod
+ if prog_ns.has_key('execfile'):
@takluyver

takluyver Mar 1, 2012

Owner

Oh, and it's generally good practice now to use 'execfile' in prog_ns rather than calling has_key.

Owner

minrk commented Mar 6, 2012

adding a note that the bug in question that this fixes is #1421.

Once the has_key -> in change is made, I think this can go in.

Owner

takluyver commented Mar 6, 2012

For the sake of completeness, I should note that if you do %run -id (i.e. run with debug in the interactive namespace), and you've defined execfile within IPython, this will remove your execfile from the namespace. I don't think that's a major concern. It shouldn't affect the standard execfile in Python 2, since that's in the builtin namespace.

Owner

takluyver commented Mar 9, 2012

Sorry, I was a bit too hasty in proposing a fix. This doesn't actually work, because our execfile expects a second argument for the namespace. Grabbing the namespace of the calling scope is a rather horrible hack.

I've worked out what we actually need to do for this. We should build a tiny new namespace with just execfile and a reference to prog_ns:

ns = {'execfile': py3compat.execfile, 'prog_ns': prog_ns}
deb.run('execfile("%s", prog_ns)' % filename, ns)

That also avoids all the potential problems from modifying the user namespace. @dchetwynd , let us know if you want to have a go at this (it would be best as a new pull request).

Owner

fperez commented Apr 15, 2012

@takluyver, it seems from your last comment that you see this particular PR as best being closed, and a new one started for the cleaner fix, am I right? I'm just trying to go through our entire backlog so we can start seeing 0.13 take shape... Let us know how we should proceed on this one.

Owner

takluyver commented Apr 15, 2012

Yep, I've made PR #1599, so I'll close this one now. Thanks @dchetwynd for getting the ball rolling, and sorry the fix I suggested didn't quite work.

@takluyver takluyver closed this Apr 15, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment