Turn NamedTemporaryFile into a context manager

[abalkin]

BPO	2021
Nosy	@ncoghlan, @abalkin, @tiran
Files	with_NamedTemporaryFile.diff: Diff against revision 60593 subclass-file.diff: diff against revision 60703

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/ncoghlan'
closed_at = <Date 2008-02-11.12:55:58.106>
created_at = <Date 2008-02-06.16:14:23.564>
labels = ['type-bug', 'library']
title = 'Turn NamedTemporaryFile into a context manager'
updated_at = <Date 2008-02-11.12:55:58.104>
user = 'https://github.com/abalkin'

bugs.python.org fields:

activity = <Date 2008-02-11.12:55:58.104>
actor = 'ncoghlan'
assignee = 'ncoghlan'
closed = True
closed_date = <Date 2008-02-11.12:55:58.106>
closer = 'ncoghlan'
components = ['Library (Lib)']
creation = <Date 2008-02-06.16:14:23.564>
creator = 'belopolsky'
dependencies = []
files = ['9360', '9401']
hgrepos = []
issue_num = 2021
keywords = ['patch']
message_count = 6.0
messages = ['62102', '62111', '62223', '62264', '62278', '62285']
nosy_count = 3.0
nosy_names = ['ncoghlan', 'belopolsky', 'christian.heimes']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue2021'
versions = ['Python 2.6']

[abalkin]

In the spirit of files becoming context managers in 2.5, the attached
patch defines __enter__ and __exit__ methods for
tempfile.NamedTemporaryFile.

BTW, I was not able to add a "patch" keyword which seems appropriate here.

[tiran]

Thanks for the patch! It even has a unit test, very good. :)

The __future__ statement isn't necessary for Python 2.6. The with
statement is always available.

[ncoghlan]

I've changed the issue type from rfe to behaviour. NamedTemporaryFile
actually provides __enter__ and __exit__ methods in 2.5, they just don't
work (it tries to use the methods from the underlying file object
directly which turns out to be a doomed exercise for a couple of
different reasons).

Fixed on the trunk in r60695. Leaving issue as pending until the
NamedTemporaryFile fix is backported to 2.5 (or we decide not to
backport it).

P.S. Alexander's patch worked as written, but in figuring out *why* it
worked I ended up moving things around a bit (main change was to only
override __exit__ when it was actually necessary to do so) and adding
some more test cases (e.g. to also cover 2.6's new SpooledTemporaryFile).

[abalkin]

Nick's comment made me think why NamedTemporaryFile can't simply
subclass file and get properly working context manager's methods for
free.

It turned out that although file is subclassable, in its present form,
it does not allow NamedTemporaryFile implementation for the following
reasons:

1. os.fdopen cannot create a subclass of file.

2. file.__exit__ does not call subclass' close method.

The attached patch fixes both issues and reimplements NamedTemporaryFile.

I understand that adding new functionality to file objects should be
brought up on python-dev, but I would like to hear comments from the
"nosy list" first.

The patch is proof-of-concept quality at the moment and may not work
non-posix platforms.

[ncoghlan]

The wrapper approach has the virtue of providing easy access to the
underlying file object and its methods. That actually gets a bit more
difficult when you switch to a subclassing approach (you need to either
explicitly qualify the methods or play around with super()).

The wrapper approach also has the virtue of being a valid candidate for
backport to 2.5.2, while that is most definitely *not* the case for
making file fully subclassable. If you would like to pursue that idea
further, I suggest opening a separate issue for it.

[ncoghlan]

Backported to 2.5 in r60728

P.S. To elaborate a bit more on why converting NamedTemporaryFile to
subclass file would be a much bigger deal than it is to just fix its
__enter__ and __exit__ methods:

• converts from old-style to new-style class
• imposes the constraints of a C base class on any subclasses
• file attribute is currently part of the public API
• need to retain wrapper approach in tempfile anyway for related type
  SpooledTemporaryFile in 2.6 (as that may contain a real file or a string
  IO object at different points in its lifecycle)
• no compelling use case for the change (the wrapper approach works,
  what real advantage do we gain from subclassing file instead?)
• wrapper approach is much easier to reconcile with Python 3.0's io module