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

fileinput, StringIO, and cStringIO do not support the with protocol #45627

Closed
ygale mannequin opened this issue Oct 16, 2007 · 18 comments
Closed

fileinput, StringIO, and cStringIO do not support the with protocol #45627

ygale mannequin opened this issue Oct 16, 2007 · 18 comments
Labels
easy extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@ygale
Copy link
Mannequin

ygale mannequin commented Oct 16, 2007

BPO 1286
Nosy @birkenfeld, @abalkin, @orsenthil, @avassalotti, @ezio-melotti, @briancurtin, @ericsnowcurrently
Files
  • with_StringIO.diff: diff against revision 60906
  • issue1286.patch: patch implementing context manager for fileinput
  • 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 = None
    closed_at = <Date 2010-07-31.20:08:39.941>
    created_at = <Date 2007-10-16.11:59:56.976>
    labels = ['extension-modules', 'easy', 'type-bug', 'library']
    title = 'fileinput, StringIO, and cStringIO do not support the with protocol'
    updated_at = <Date 2012-11-28.07:00:51.449>
    user = 'https://bugs.python.org/ygale'

    bugs.python.org fields:

    activity = <Date 2012-11-28.07:00:51.449>
    actor = 'eric.snow'
    assignee = 'none'
    closed = True
    closed_date = <Date 2010-07-31.20:08:39.941>
    closer = 'georg.brandl'
    components = ['Extension Modules', 'Library (Lib)']
    creation = <Date 2007-10-16.11:59:56.976>
    creator = 'ygale'
    dependencies = []
    files = ['9470', '15355']
    hgrepos = []
    issue_num = 1286
    keywords = ['patch', 'easy']
    message_count = 18.0
    messages = ['56497', '56636', '56643', '56678', '56714', '56740', '56757', '59778', '62595', '95401', '96699', '96944', '112195', '176329', '176361', '176512', '176513', '176518']
    nosy_count = 9.0
    nosy_names = ['georg.brandl', 'belopolsky', 'ygale', 'orsenthil', 'alexandre.vassalotti', 'ezio.melotti', 'brian.curtin', 'eric.snow', 'Wade.Tattersall']
    pr_nums = []
    priority = 'low'
    resolution = 'accepted'
    stage = 'patch review'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue1286'
    versions = ['Python 2.7']

    @ygale ygale mannequin added extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error labels Oct 16, 2007
    @ygale
    Copy link
    Mannequin Author

    ygale mannequin commented Oct 16, 2007

    The standard idiom for opening a file is now with open...
    So I think it should be a goal that this should work with
    any built-in file-like object that needs to be closed,
    without having to explicitly wrap it in closing().
    It certainly should work for fileinput and StringIO - since
    these really are files, in some sense.

    @avassalotti
    Copy link
    Member

    Do you have a use-case for this? In Py3k, I don't think adding support
    for the 'with' statement to StringIO makes any sense, since the close()
    method does nothing.

    @ygale
    Copy link
    Mannequin Author

    ygale mannequin commented Oct 22, 2007

    These objects are supposed to be drop-in replacements
    for file handles. Except in legacy code, the way you
    use a file handle is:

    with function_to_create_fh as fh:
      <do stuff>

    If these objects do not support the with protocol,
    the only way to use them is to refactor this code.
    That may not even be possible, e.g., if it is in
    a library, or it may not be desirable to refactor.

    Even if you can refactor it, I don't think you
    can call these objects file-like objects if
    you can't use them like a file.

    Note that in legacy code, you used to write:

    fh = function_to_get_fh
    try:
      <do stuff>
    finally:
      fh.close()

    If function_to_get_fh happens to return some other
    file-like object and not an actual file,
    this still works fine without any refactoring.

    You wrote:

    In Py3k, I don't think adding support
    for the 'with' statement to StringIO makes
    any sense, since the close()
    method does nothing.

    So do you propse removing the close() method
    altogether? Of course the answer is "no",
    because then you could not use the object like
    a file. The same is true for the with protocol.

    (I now retract the words "that needs to be closed"
    from my original comment. All file-like objects
    should support the with protocol, regardles of
    whether their close() method actually does anything.)

    @birkenfeld
    Copy link
    Member

    Makes sense to me.

    @gvanrossum
    Copy link
    Member

    Let's not get overexcited. I agree that it makes sense for fileinput,
    but I disagree about *StringIO; its close() isn't needed to free
    resources (since it doesn't use up a scarce resource like a file
    descriptor).

    Can you whip up a patch for fileinput? Please include unit tests and
    documentation.

    @ygale
    Copy link
    Mannequin Author

    ygale mannequin commented Oct 25, 2007

    I was actually bitten badly by this issue with
    StringIO. I added fileinput only as an afterthought.

    In an xml.sax app, I needed seek() support for a
    codec-wrapped file handle, so I over-wrapped it with
    StringIO. The result was that I had to refactor code all over
    the place to handle StringIO as a special case. What a
    mess!

    Why is this getting over-excited? It's a very
    lightweight change. You can't beat the cost/benefit ratio.

    @gvanrossum
    Copy link
    Member

    2007/10/25, Yitz Gale <report@bugs.python.org>:

    I was actually bitten badly by this issue with
    StringIO. I added fileinput only as an afterthought.

    In an xml.sax app, I needed seek() support for a
    codec-wrapped file handle, so I over-wrapped it with
    StringIO. The result was that I had to refactor code all over
    the place to handle StringIO as a special case. What a
    mess!

    I don't understand. What did your code look like after the refactoring?

    I find that typically a useful idiom is to have one piece of code
    handle opening/closing of streams and let the rest of the code just
    deal with streams without ever closing them. E.g.

    f = open(filename)
    try:
      process(f)
    finally:
      f.close()

    or, if you want:

    with open(filename) as f:
      process(f)

    As I don't understand how you are working the StringIO() call into
    this I'm still not sure what the issue is.

    Why is this getting over-excited? It's a very
    lightweight change. You can't beat the cost/benefit ratio.

    Until you submit a patch it's more work for me. :-)

    @akuchling akuchling added the easy label Jan 12, 2008
    @avassalotti
    Copy link
    Member

    FYI, StringIO and BytesIO, in Python 3K, already support the context
    management protocol.

    @abalkin
    Copy link
    Member

    abalkin commented Feb 20, 2008

    Attached patch implements context management protocol for StringIO.

    @abalkin abalkin added the stdlib Python modules in the Lib dir label Feb 20, 2008
    @briancurtin
    Copy link
    Member

    Attached is a patch against trunk r76325 which implements context
    manager support for fileinput/FileInput, with tests and doc change.

    @orsenthil
    Copy link
    Member

    I reviewed the patches attached.

    • The patch to add Context Manager support for fileinput.py seems good.
      It has docs too.

    This discussion did not conclude on the need for Context Manager for
    StringIO. With py3k having it, it should be good for py2.7 to provide
    the support too. The attached patch seems good enough, Docs can be added
    further.

    @birkenfeld
    Copy link
    Member

    FWIW, for the sake of consistency I'm +1 on supporting the context
    manager protocol on all file-like objects in the stdlib.

    @birkenfeld
    Copy link
    Member

    For 3.x, the builtin io.StringIO and io.BytesIO already have context manager capability. Added fileinput in r83359.

    @WadeTattersall
    Copy link
    Mannequin

    WadeTattersall mannequin commented Nov 25, 2012

    Any chance this patch could be applied to version 2.7? It's still an issue in 2.7.3, even though a suitable patch was supplied 3 years ago.

    I understand that it's fixed in python3, but for us poor maintainers of ancient code, it would be convenient to be able to do things like

    with StringIO() as test:
        test.write("hi!")
        return test.getvalue()

    @birkenfeld
    Copy link
    Member

    It would be a new feature, and as such forbidden to add in maintenance releases.

    @ericsnowcurrently
    Copy link
    Member

    Keep in mind that it's pretty easy to roll your own CM wrapper:

    @contextlib.contextmanager
    def closes(file):
        yield file
        file.close()

    Then you can do this:

    with closes(StringIO()) as test:
        test.write("hi!")
        return test.getvalue()

    This works for 2.5 and up.

    @ezio-melotti
    Copy link
    Member

    @ericsnowcurrently
    Copy link
    Member

    borrowed the time machine did we? ;)

    @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
    easy extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    9 participants