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

add os.close() suggestion to mkstemp documentation #51124

Closed
vincele mannequin opened this issue Sep 10, 2009 · 6 comments
Closed

add os.close() suggestion to mkstemp documentation #51124

vincele mannequin opened this issue Sep 10, 2009 · 6 comments
Assignees
Labels
docs Documentation in the Doc dir type-feature A feature request or enhancement

Comments

@vincele
Copy link
Mannequin

vincele mannequin commented Sep 10, 2009

BPO 6875
Nosy @birkenfeld, @bitdancer
Files
  • python-doc-mkstemp.patch: patch adding os.close() suggestion to the mkstemp documentation
  • 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/birkenfeld'
    closed_at = <Date 2009-09-16.13:04:02.173>
    created_at = <Date 2009-09-10.11:47:03.911>
    labels = ['type-feature', 'docs']
    title = 'add os.close() suggestion to mkstemp documentation'
    updated_at = <Date 2013-08-13.14:41:21.372>
    user = 'https://bugs.python.org/vincele'

    bugs.python.org fields:

    activity = <Date 2013-08-13.14:41:21.372>
    actor = 'Gabi.Davar'
    assignee = 'georg.brandl'
    closed = True
    closed_date = <Date 2009-09-16.13:04:02.173>
    closer = 'georg.brandl'
    components = ['Documentation']
    creation = <Date 2009-09-10.11:47:03.911>
    creator = 'vincele'
    dependencies = []
    files = ['14870']
    hgrepos = []
    issue_num = 6875
    keywords = ['patch']
    message_count = 6.0
    messages = ['92477', '92479', '92533', '92537', '92549', '92684']
    nosy_count = 4.0
    nosy_names = ['georg.brandl', 'r.david.murray', 'vincele', 'Gabi.Davar']
    pr_nums = []
    priority = 'normal'
    resolution = 'works for me'
    stage = None
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue6875'
    versions = ['Python 2.6', 'Python 3.1', 'Python 2.7', 'Python 3.2']

    @vincele
    Copy link
    Mannequin Author

    vincele mannequin commented Sep 10, 2009

    As per the blog entry
    http://www.logilab.org/blogentry/17873

    I think the tempfile.mkstemp() documentation could be more helpful by
    suggesting the use of os.close() appropriately.

    If some native english speaker could give a review of the language I
    used in the additional text, I'd be grateful.

    @vincele vincele mannequin assigned birkenfeld Sep 10, 2009
    @vincele vincele mannequin added docs Documentation in the Doc dir type-feature A feature request or enhancement labels Sep 10, 2009
    @vincele
    Copy link
    Mannequin Author

    vincele mannequin commented Sep 10, 2009

    Or a review of the markup I used

    @bitdancer
    Copy link
    Member

    I don't think it would be beneficial to just talk about closing the file
    descriptor, or to talk about os open file limits. Closing open files
    when they are no longer needed is just good programming practice and IMO
    a discussion of it doesn't belong in the library docs unless it is at a
    more generic level (like os.open or open).

    I don't think the code shown in your blog post is using the library the
    way it is designed. If the programmer wants an open file object, they
    should use TemporaryFile or NamedTemporaryFile. If there really is a
    reason to get an os file handle and turn it into a file object, then
    os.openfd should be used. Perhaps the appropriate change to the
    documentation would be to mention os.openfd, and to mention that
    os.close is the correct function to use to close the file handle when it
    is no longer needed (unless, of course, openfd has been called).

    As for your markup, your ``os.close`` should instead be
    :func:`os.close`, which results in the automatic creation of a link to
    the os.close documentation.

    @vincele
    Copy link
    Mannequin Author

    vincele mannequin commented Sep 12, 2009

    The real question I had is why mkstemp return an os-level opened file
    descriptor instead of a python file object...

    @bitdancer
    Copy link
    Member

    Because if you want the Python file object, you should use TemporaryFile
    or NamedTemporaryFile. mkstemp is a lower level (closer to the os)
    interface, and is provided because Python has a philosophy of providing
    those low level interfaces when possible. Note that the TemporaryFile
    classes use mkstemp in their implementation.

    @birkenfeld
    Copy link
    Member

    That one has to close open files should be common knowledge.

    And it's already documented that the filehandle returned is to be
    treated as if coming from os.open, so the "isn't a file object" is
    documented as well.

    Insofar, I'm in agreement with David.

    @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
    docs Documentation in the Doc dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants