Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert #1831, the __file__ injection in safe_execfile / safe_execfile_ipy. #2432

Merged
merged 1 commit into from Sep 30, 2012

Conversation

bfroehle
Copy link
Contributor

This reverts commit 2717feb, reversing changes made to ea4f608.

Pull request #1831 (fix #1814 set __file__ when running .ipy files) has been the source of a lot of grief:

In general the patch was inappropriate because it:

  1. Fails to properly restore the context, by setting __file__ to None rather than deleting it.
  2. Sets __file__ in the wrong dictionary (self.user_ns rather than where[0]).

I suggest that we revert pull request #1831 and reopen the original issue (#1814).

This reverts commit 2717feb, reversing
changes made to ea4f608.
@takluyver
Copy link
Member

I haven't been following this one, but is there an easy way to add a test, so we don't break the same thing next time we try to do that?

@bfroehle
Copy link
Contributor Author

Yes, I think so. I've been working on a better solution in bfroehle/ipy_file_inject.

@bfroehle
Copy link
Contributor Author

Well, some tests are easy to write, like verifying that __file__ doesn't leak out of a call to %run. On the other hand how to test that __file__ is not leaked out of ipython -i <filename> is a bit less obvious to me.

@takluyver
Copy link
Member

It's possible using pexpect, but probably not worth it.

@jstenar
Copy link
Member

jstenar commented Sep 26, 2012

It sounds like this issue might be related as well #2356

@bfroehle
Copy link
Contributor Author

Without objection, I'm planning on merging this tomorrow. I got hit with the Mayavi bug (#2279) yesterday when doing a demo.

@Carreau
Copy link
Member

Carreau commented Sep 30, 2012

Merging.

Carreau added a commit that referenced this pull request Sep 30, 2012
Revert #1831, the `__file__` injection in safe_execfile / safe_execfile_ipy.

This reverts commit 2717feb, reversing changes made to ea4f608.

Pull request #1831 (fix #1814 set __file__ when running .ipy files) has been the source of a lot of grief:

#2279: Setting __file__ to None breaks Mayavi import
#2429: Using warnings.warn() results in TypeError
In general the patch was inappropriate because it:
1. Fails to properly restore the context, by setting __file__ to None rather than deleting it.
2. Sets __file__ in the wrong dictionary (self.user_ns rather than where[0]).
@Carreau Carreau merged commit d3d37a3 into ipython:master Sep 30, 2012
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Revert ipython#1831, the `__file__` injection in safe_execfile / safe_execfile_ipy.

This reverts commit 2717feb, reversing changes made to ea4f608.

Pull request ipython#1831 (fix ipython#1814 set __file__ when running .ipy files) has been the source of a lot of grief:

ipython#2279: Setting __file__ to None breaks Mayavi import
ipython#2429: Using warnings.warn() results in TypeError
In general the patch was inappropriate because it:
1. Fails to properly restore the context, by setting __file__ to None rather than deleting it.
2. Sets __file__ in the wrong dictionary (self.user_ns rather than where[0]).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

__file__ is not defined when file end with .ipy
4 participants