This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: add `copy_from` argument to temporaryfile
Type: enhancement Stage:
Components: Library (Lib) Versions: Python 3.4
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: kyle.roberts, ncoghlan, pitrou, r.david.murray
Priority: normal Keywords: patch

Created on 2013-04-09 10:19 by pitrou, last changed 2022-04-11 14:57 by admin.

Files
File name Uploaded Description Edit
copy_from.patch kyle.roberts, 2013-04-17 03:54 review
copy_from_test.txt kyle.roberts, 2013-04-17 03:55
copy_from_v2.patch kyle.roberts, 2013-04-17 04:14 review
Messages (11)
msg186392 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-04-09 10:19
It is sometimes useful to create a temporary file as a copy of an existing file (especially e.g. in tests, when you want to mutate the temp file but not the original). I would suggest adding a `copy_from` argument to TemporaryFile and NamedTemporaryFile, which is either the path to an existing file or a file-like object. Then the contents of that source file are copied to the tempfile before it is yielded.
msg186633 - (view) Author: Kyle Roberts (kyle.roberts) Date: 2013-04-12 14:07
I'm working on a patch for this.
msg187000 - (view) Author: Kyle Roberts (kyle.roberts) Date: 2013-04-15 15:25
I think `copy_from` should be included for mkstemp as well. It provides similar functionality to TemporaryFile and NamedTemporaryFile, but it doesn't delete the temp file on close as the other two do by default. Thoughts?
msg187136 - (view) Author: Kyle Roberts (kyle.roberts) Date: 2013-04-17 03:54
Attached is a patch to include the copy_from argument in mkstemp, TemporaryFile, and NamedTemporaryFile. Also attached is a text file for testing the copy operation.

Some notes:

I intended to use shutil's copyfile(src, dst) method to copy the copy_from file to the temp file, but copyfile implicitly closes the dst file (our temp file), which would lead to deleting the temp file. I created a simple private _copyfile(src, dst) that keeps the dst file open after writing to it. One question I have regarding my _copyfile is should I include the check for a named pipe as was done in shutil.copyfile? I think that the same issue applies as described in http://bugs.python.org/issue3002.

Another thing I wasn't sure of is the best way to include a test file. The copy_from tests included in the patch include a txt file, but the way I went about getting it seems wonky. Is there a best practice for this?

Finally, the copy_from parameter only takes a string filepath argument. I thought it might be better to avoid handling two different types (string or file-like object) for copying because that is how other copy operations (i.e. copyfile()) work.
msg187139 - (view) Author: Kyle Roberts (kyle.roberts) Date: 2013-04-17 04:14
Sorry, disregard my second question about finding a test file. I found an example in test_mailcap.py. I've uploaded a new patch with the cleaner test functions.
msg187169 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-04-17 14:57
Personally I prefer having the test create the needed test file, rather than adding a new file to the repository, for small files.  That's just my opinion, though; as you found what you did is common practice currently.  Your patch doesn't include the file, though.

(I haven't reviewed the rest of the patch.)
msg187170 - (view) Author: Kyle Roberts (kyle.roberts) Date: 2013-04-17 15:08
Thanks for the quick response. The test file is sandwiched in between the two patch files, so it's easy to miss. You made a great point about not cluttering up the repo with any unnecessary files. I'll change the test methods to create a new copy_from file that's closed at the end of each test.
msg187171 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-04-17 15:28
I did miss it.  Not that it matters if you are going to rewrite the test, but FYI you can include new files in a patch by adding them ('hg add') before doing the 'hg diff' to generate the patch.
msg187179 - (view) Author: Kyle Roberts (kyle.roberts) Date: 2013-04-17 16:45
Very cool, I didn't think it'd be included in the patch for some reason. Thanks!
msg187188 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-04-17 18:25
Some comments about the patch:
- unless I'm missing something, I think you should re-use the fd instead of reopening the file by name
- it would be nice if copy_from accepted an open file-like object, e.g. a BytesIO instance
- I don't think you need a specific file for the test; just use an existing one (e.g. __file__, or one of the data files somewhere in the test subtree)
- the doc needs some "versionchanged" markers
msg188073 - (view) Author: Kyle Roberts (kyle.roberts) Date: 2013-04-29 15:11
Thanks for the comments Antoine. I didn't have a good reason for using the file name at the time, although even when using the file name I should have used the "file" variable that was already created. I tried using the file descriptor, but I encountered a "[Errno 9] Bad file descriptor" once the methods using _mkstemp_inner call _io.open on the returned fd. This leads me to believe that the fd is being closed somehow, but as you can see from my copy function, I don't implicitly or explicitly close the file using the file descriptor. I'm not sure yet why the file name works as expected but the fd does not. Am I missing something simple?

As for points 2-4 I have most of that done. What's the pythonic way for determining if an argument is file-like? I've seen isinstance, hasattr, etc.
History
Date User Action Args
2022-04-11 14:57:44adminsetgithub: 61873
2013-04-29 15:11:28kyle.robertssetmessages: + msg188073
2013-04-17 18:25:17pitrousetmessages: + msg187188
2013-04-17 16:45:46kyle.robertssetmessages: + msg187179
2013-04-17 15:28:06r.david.murraysetmessages: + msg187171
2013-04-17 15:08:11kyle.robertssetmessages: + msg187170
2013-04-17 14:57:16r.david.murraysetnosy: + r.david.murray
messages: + msg187169
2013-04-17 04:14:17kyle.robertssetfiles: + copy_from_v2.patch

messages: + msg187139
2013-04-17 03:55:20kyle.robertssetfiles: + copy_from_test.txt
2013-04-17 03:54:55kyle.robertssetfiles: + copy_from.patch
keywords: + patch
messages: + msg187136
2013-04-15 15:25:12kyle.robertssetmessages: + msg187000
2013-04-12 14:07:57kyle.robertssetnosy: + kyle.roberts
messages: + msg186633
2013-04-09 10:19:58pitroucreate