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

avoid import of nearby temporary with %edit #4848

Merged
merged 1 commit into from Jan 30, 2014

Conversation

Carreau
Copy link
Member

@Carreau Carreau commented Jan 22, 2014

use mkstemp now,
and create files in subfolders.

should close gh-4731

Note that some part of the logic could now use
http://docs.python.org/2/library/tempfile.html#tempfile.NamedTemporaryFile
That will be deleted on close and avoid IPython to track it.

use mk**s**temp now,
and create files in subfolders.

should close ipythongh-4731

Note that some part of the logic could now use
http://docs.python.org/2/library/tempfile.html#tempfile.NamedTemporaryFile
That will be deleted on close and avoid IPython to track it.
@ivanov
Copy link
Member

ivanov commented Jan 22, 2014

code looks clean. Do you think this is worth having a test of the edit magic? otherwise, 👍

for tfile in self.tempfiles:
try:
os.unlink(tfile)
except OSError:
pass

for tdir in self.tempdirs:
try:
os.rmdir(tdir)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will fail if there's anything left in the directory. I think that's fine, because temporary files should be cleaned up anyway, I just wanted to note it.

@takluyver
Copy link
Member

Unfortunately, NamedTemporaryFile is next to useless, because on Windows, you can't use the name to open the file while it's open in the Python process.

@ivanov
Copy link
Member

ivanov commented Jan 23, 2014

what about using from IPython.utils.tempdir import NamedFileInTemporaryDirectory here?

@Carreau
Copy link
Member Author

Carreau commented Jan 23, 2014

what about using from IPython.utils.tempdir import NamedFileInTemporaryDirectory here?

Ahhhh ! Will look at that.

@Carreau
Copy link
Member Author

Carreau commented Jan 23, 2014

IPython.utils.tempdir import NamedFileInTemporaryDirectory act much more as a context manager and has quite a different behavior than mktemp. In particular it opens the file and wait to have cleanup called on it which close the file.
Which I don't want as mktemp just create a file with data and close it. The will be reopened at a later point by the editor.

I could used it, but it would require quite some change / addition :
Option not to keep the file open, ability to get file abspath, ability to pass some data to write to the file.

Using it as a context manager could be doable, but would require significant rework of the %edit magic and _find_edit_target.

So I'm not 100% sure it is worth.

@ghost ghost assigned ellisonbg Jan 23, 2014
takluyver added a commit that referenced this pull request Jan 30, 2014
avoid import of nearby temporary with %edit
@takluyver takluyver merged commit 6019921 into ipython:master Jan 30, 2014
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
avoid import of nearby temporary with %edit
@Carreau Carreau deleted the closes-4731 branch December 15, 2014 16:47
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.

%edit files mistakenly import modules in /tmp
4 participants