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

Jump to bottom

Make mmap.write return the number of bytes written like other write methods #70523

Closed
jstasiak mannequin opened this issue Feb 10, 2016 · 8 comments
Closed

Make mmap.write return the number of bytes written like other write methods #70523

jstasiak mannequin opened this issue Feb 10, 2016 · 8 comments
Labels
extension-modules C modules in the Modules dir type-feature A feature request or enhancement

Comments

@jstasiak
Copy link
Mannequin

jstasiak mannequin commented Feb 10, 2016

BPO 26335
Nosy @Yhg1s, @berkerpeksag, @vadmium, @jstasiak
Files
  • mmap_write_return_count.patch
  • mmap_write_return_count2.patch
  • mmap_write_return_count3.patch
    		•

    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 2016-03-02.17:31:09.579>
created_at = <Date 2016-02-10.22:55:37.908>
labels = ['extension-modules', 'type-feature']
title = 'Make mmap.write return the number of bytes written like other write methods'
updated_at = <Date 2016-03-03.00:02:26.588>
user = 'https://github.com/jstasiak'

    bugs.python.org fields:

    activity = <Date 2016-03-03.00:02:26.588>
actor = 'jstasiak'
assignee = 'none'
closed = True
closed_date = <Date 2016-03-02.17:31:09.579>
closer = 'berker.peksag'
components = ['Extension Modules']
creation = <Date 2016-02-10.22:55:37.908>
creator = 'jstasiak'
dependencies = []
files = ['41890', '42014', '42026']
hgrepos = []
issue_num = 26335
keywords = ['patch']
message_count = 8.0
messages = ['260053', '260706', '260750', '260759', '260847', '261128', '261129', '261143']
nosy_count = 5.0
nosy_names = ['twouters', 'python-dev', 'berker.peksag', 'martin.panter', 'jstasiak']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue26335'
versions = ['Python 3.6']
    @jstasiak
    Copy link
    Mannequin Author

    jstasiak mannequin commented Feb 10, 2016

    Since mmap objects are said to "behave like both bytearray and like file objects" I believe it's appropriate for the mmap.write() method to return the number of bytes written like write() of other file objects/interfaces I could find in the standard library for consistency reasons:

    https://docs.python.org/3/library/io.html#io.BufferedIOBase.write
    https://docs.python.org/3/library/io.html#io.BufferedWriter.write
    https://docs.python.org/3/library/io.html#io.RawIOBase.write
    https://docs.python.org/3/library/io.html#io.TextIOBase.write

    Why I believe this would be useful: code that writes to file objects and tests the number of bytes/characters written right now will likely fail when it's passed a mmap object because its write() method returns None. With this patch applied it'll work transparently.

    Please find proposed patch attached, I included information about the exception type in the documentation as it seems fitting (apologies for generating the patch using Git, I'll generate using Mercurial if necessary).

    @jstasiak jstasiak mannequin added stdlib Python modules in the Lib dir topic-IO type-feature A feature request or enhancement labels Feb 10, 2016
    @berkerpeksag
    Copy link
    Member

    Thanks for the patch, Jakub. I don't use mmap module much so I don't have an opinion about the feature, but it sounds reasonable.

    I left some review comments on Rietveld: http://bugs.python.org/review/26335/

    @berkerpeksag berkerpeksag added extension-modules C modules in the Modules dir and removed stdlib Python modules in the Lib dir topic-IO labels Feb 23, 2016
    @jstasiak
    Copy link
    Mannequin Author

    jstasiak mannequin commented Feb 23, 2016

    Oops, sorry for the silliness in the C code, thanks for reviewing. I modified as suggested, please find the new patch attached.

    @vadmium
    Copy link
    Member

    vadmium commented Feb 24, 2016

    Patch looks okay to me. I guess it would be good to write a What’s New entry as well.

    @jstasiak
    Copy link
    Mannequin Author

    jstasiak mannequin commented Feb 25, 2016

    Thank you. I didn't know whether to add an entry to Doc/whatsnew/3.6.rst, Misc/NEWS or both so I chose both, feel free to modify/remove as needed.

    The new patch also doesn't have a typo in the versionchanged directive present in the version 2. I noticed more typos like this (single colon instead of double colon), I'll create a separate issue.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 2, 2016

    New changeset ba71aecec943 by Berker Peksag in branch 'default':
    Issue bpo-26335: Make mmap.write() return the number of bytes written like
    https://hg.python.org/cpython/rev/ba71aecec943

    @berkerpeksag
    Copy link
    Member

    Thanks for the patch, Jakub!

    @jstasiak
    Copy link
    Mannequin Author

    jstasiak mannequin commented Mar 3, 2016

    Glad I could help, thanks for merging!

    @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
    Assignees
    No one assigned
    Labels
    extension-modules C modules in the Modules dir type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @berkerpeksag @vadmium