Changes by Jesús Cea Avión j...@jcea.es:
--
nosy: +jcea
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12021
___
___
Python-bugs-list mailing list
Antoine Pitrou pit...@free.fr added the comment:
Patch looks good to me, thank you :)
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12021
___
Roundup Robot devnull@devnull added the comment:
New changeset 964d0d65a2a9 by Charles-François Natali in branch 'default':
Issue #12021: Make mmap's read() method argument optional. Patch by Petri
http://hg.python.org/cpython/rev/964d0d65a2a9
--
nosy: +python-dev
Charles-François Natali neolo...@free.fr added the comment:
Patch committed.
Thanks for the report and the patch!
--
resolution: - fixed
stage: patch review - committed/rejected
status: open - closed
___
Python tracker rep...@bugs.python.org
Petri Lehtinen pe...@digip.org added the comment:
It seems I did. Attached now for real :)
--
Added file: http://bugs.python.org/file22275/mmap_read_all_4.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12021
Charles-François Natali neolo...@free.fr added the comment:
That's because of the _PyIO_ConvertSsize_t converter, which silently
converts None to -1.
There's probably a good reason for doing this in the _io module
I'm not sure about the original reason, but I find None as the default
Antoine Pitrou pit...@free.fr added the comment:
If being pretty is the only reason for this choice, then I think that
documenting the method as
method:: read([n])
is simpler and cleaner .
But you've got much more experience than me, so I won't argue any further :-)
There are
Petri Lehtinen pe...@digip.org added the comment:
I think duplicating it is fine, since the code is probably simple anyway
Added a new patch. I duplicated the None converting logic in _io/_iomodule.c to
mmapmodule.c, changed the doc to say read([n]), and expanded the test cases a
bit.
Antoine Pitrou pit...@free.fr added the comment:
Didn't you forget to attach the patch?
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12021
___
Antoine Pitrou pit...@free.fr added the comment:
That's because of the _PyIO_ConvertSsize_t converter, which silently
converts None to -1.
There's probably a good reason for doing this in the _io module
I'm not sure about the original reason, but I find None as the default
omitted value
Petri Lehtinen pe...@digip.org added the comment:
Antoine Pitrou wrote:
I'm not sure about the original reason, but I find None as the default
omitted value prettier than -1 myself, so I think it's a good thing :)
Yeah, and it's also good for consistency with other file-like objects.
Can I
Petri Lehtinen pe...@digip.org added the comment:
I noticed that RawIOBase.read, TextIOBase.read, etc also accept None as the
argument, treating it as -1. Should this be implemented, too?
--
___
Python tracker rep...@bugs.python.org
Charles-François Natali neolo...@free.fr added the comment:
I noticed that RawIOBase.read, TextIOBase.read, etc also accept None as the
argument, treating it as -1. Should this be implemented, too?
That's because of the _PyIO_ConvertSsize_t converter, which silently
converts None to -1.
Charles-François Natali neolo...@free.fr added the comment:
The patch looks good to me.
In your test, you don't explicitely close the mmap object: it's not a problem
with CPython since it will get unmmapped, and the file descriptor - if it's a
file-backed mapping - will get closed, as soon as
Petri Lehtinen pe...@digip.org added the comment:
Thanks for the comments. I attached a new patch that uses
self.addCleanup(m.close), and also adds a versionchanged directive to the docs.
--
Added file: http://bugs.python.org/file22199/mmap_read_all_3.patch
Petri Lehtinen pe...@digip.org added the comment:
Added a patch. It was only a matter of making the size parameter optional.
--
keywords: +patch
nosy: +petri.lehtinen
Added file: http://bugs.python.org/file22142/mmap_read_all.patch
___
Python tracker
Petri Lehtinen pe...@digip.org added the comment:
Updated the patch to also update the documentation of mmap.read().
--
Added file: http://bugs.python.org/file22143/mmap_read_all_2.patch
___
Python tracker rep...@bugs.python.org
Changes by John O'Connor tehj...@gmail.com:
--
nosy: +jcon
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12021
___
___
Python-bugs-list mailing
New submission from K Richard Pixley r...@noir.com:
mmap.read requires a argument. Since most file-like objects do not, this
breaks the file-like object illusion.
mmap.read argument should be optional, presumably defaulting to the entire
mmap'd area.
--
messages: 135362
nosy:
Antoine Pitrou pit...@free.fr added the comment:
Sounds like a reasonable request, although I don't think it's a bug fix in
itself (there are probably other places where mmap breaks the file-like
expectation).
--
nosy: +pitrou
stage: - needs patch
type: behavior - feature request
Changes by Nadeem Vawda nadeem.va...@gmail.com:
--
nosy: +nadeem.vawda
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12021
___
___
21 matches
Mail list logo
