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

Turn NamedTemporaryFile into a context manager #46305

Closed
abalkin opened this issue Feb 6, 2008 · 6 comments
Closed

Turn NamedTemporaryFile into a context manager #46305

abalkin opened this issue Feb 6, 2008 · 6 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@abalkin
Copy link
Member

abalkin commented Feb 6, 2008

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
    Copy link
    Member Author

    abalkin commented Feb 6, 2008

    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.

    @abalkin abalkin added type-bug An unexpected behavior, bug, or error stdlib Python modules in the Lib dir labels Feb 6, 2008
    @tiran
    Copy link
    Member

    tiran commented Feb 6, 2008

    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.

    @tiran tiran added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Feb 6, 2008
    @ncoghlan ncoghlan self-assigned this Feb 9, 2008
    @ncoghlan
    Copy link
    Contributor

    ncoghlan commented Feb 9, 2008

    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).

    @ncoghlan ncoghlan added type-bug An unexpected behavior, bug, or error and removed type-feature A feature request or enhancement labels Feb 9, 2008
    @abalkin
    Copy link
    Member Author

    abalkin commented Feb 10, 2008

    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
    Copy link
    Contributor

    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
    Copy link
    Contributor

    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

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants