-
-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
Make mmap.write return the number of bytes written like other write methods #70523
Comments
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 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). |
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/ |
Oops, sorry for the silliness in the C code, thanks for reviewing. I modified as suggested, please find the new patch attached. |
Patch looks okay to me. I guess it would be good to write a What’s New entry as well. |
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. |
New changeset ba71aecec943 by Berker Peksag in branch 'default': |
Thanks for the patch, Jakub! |
Glad I could help, thanks for merging! |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: