classification
Title: Make mmap.write return the number of bytes written like other write methods
Type: enhancement Stage: resolved
Components: Extension Modules Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: berker.peksag, jstasiak, martin.panter, python-dev, twouters
Priority: normal Keywords: patch

Created on 2016-02-10 22:55 by jstasiak, last changed 2016-03-03 00:02 by jstasiak. This issue is now closed.

Files
File name Uploaded Description Edit
mmap_write_return_count.patch jstasiak, 2016-02-10 22:55 review
mmap_write_return_count2.patch jstasiak, 2016-02-23 23:39 review
mmap_write_return_count3.patch jstasiak, 2016-02-25 10:05
Messages (8)
msg260053 - (view) Author: Jakub Stasiak (jstasiak) * Date: 2016-02-10 22:55
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).
msg260706 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-02-23 00:02
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/
msg260750 - (view) Author: Jakub Stasiak (jstasiak) * Date: 2016-02-23 23:39
Oops, sorry for the silliness in the C code, thanks for reviewing. I modified as suggested, please find the new patch attached.
msg260759 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-02-24 02:28
Patch looks okay to me. I guess it would be good to write a What’s New entry as well.
msg260847 - (view) Author: Jakub Stasiak (jstasiak) * Date: 2016-02-25 10:05
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.
msg261128 - (view) Author: Roundup Robot (python-dev) Date: 2016-03-02 17:29
New changeset ba71aecec943 by Berker Peksag in branch 'default':
Issue #26335: Make mmap.write() return the number of bytes written like
https://hg.python.org/cpython/rev/ba71aecec943
msg261129 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-03-02 17:31
Thanks for the patch, Jakub!
msg261143 - (view) Author: Jakub Stasiak (jstasiak) * Date: 2016-03-03 00:02
Glad I could help, thanks for merging!
History
Date User Action Args
2016-03-03 00:02:26jstasiaksetmessages: + msg261143
2016-03-02 17:31:09berker.peksagsetstatus: open -> closed
resolution: fixed
messages: + msg261129

stage: patch review -> resolved
2016-03-02 17:29:56python-devsetnosy: + python-dev
messages: + msg261128
2016-02-25 10:05:33jstasiaksetfiles: + mmap_write_return_count3.patch

messages: + msg260847
2016-02-24 02:28:37martin.pantersetnosy: + martin.panter
messages: + msg260759
2016-02-23 23:39:34jstasiaksetfiles: + mmap_write_return_count2.patch

messages: + msg260750
2016-02-23 00:02:13berker.peksagsetnosy: + berker.peksag
messages: + msg260706

components: + Extension Modules, - Library (Lib), IO
stage: patch review
2016-02-12 23:23:45terry.reedysetnosy: + twouters
2016-02-10 22:55:37jstasiakcreate