bpo-31368: Enhance os.preadv() documentation#5415
bpo-31368: Enhance os.preadv() documentation#5415vstinner wants to merge 5 commits intopython:masterfrom vstinner:preadv_doc
Conversation
|
@pablogsal: would you mind to review my PR? |
| Write the *buffers* contents to file descriptor *fd* at offset *offset*. | ||
| *buffers* must be a sequence of :term:`bytes-like objects <bytes-like | ||
| object>`. Buffers are processed in array order. Entire contents of first | ||
| buffer is written before proceeding to second, and so on. |
There was a problem hiding this comment.
'Entire contents of the first buffer'
'before proceeding to the second'
| (sysconf() value SC_IOV_MAX) on the number of buffers that can be used. | ||
| processed in array order. Entire contents of the first buffer is written | ||
| before proceeding to the second, and so on. The operating system may set a | ||
| limit (sysconf() value SC_IOV_MAX) on the number of buffers that can be |
There was a problem hiding this comment.
It will make sense to change
sysconf() -> :func:`sysconf`
and
SC_IOV_MAX -> ``'SC_IOV_MAX'``
?
There was a problem hiding this comment.
Oh wait, I didn't want to modify this documentation... But then I read the doc, and oh, all these documentations can be enhanced...
pread(): rename second parameter from 'buffersize' to 'n', reuse read() parameter name. The parameter is positional-only and so this change is backward compatible.
|
Sorry for reviewers, but I basically rewrote my whole PR to cleanup the documentation of all read and write functions:
The documentation should now be more consistent. |
| - :data:`RWF_HIPRI` | ||
| - :data:`RWF_NOWAIT` | ||
|
|
||
| Using non-zero flags requires Linux 4.6 or newer. |
There was a problem hiding this comment.
could be change this to: "Using non-zero flags requires preadv2 support on Linux 4.7 or newer."
There was a problem hiding this comment.
I'm trying to avoid mentioning preadv2(). Do you think that you must mention it?
There was a problem hiding this comment.
All the flags actually belong to preadv2() function though, maybe a tiny mention wont hurt?
Its your call.
There was a problem hiding this comment.
You should absolutely mention preadv2 and pwritev2. At the point someone is using these features in the os module they're also reading man pages and lwn articles, and you want to help them make the connection. If someone wonders whether python has a wrapper for preadv2 then searching for that in the docs should find os.preadv.
| - :data:`RWF_DSYNC` | ||
| - :data:`RWF_SYNC` | ||
|
|
||
| Using non-zero flags requires Linux 4.7 or newer. |
There was a problem hiding this comment.
could be also change this to: "Using non-zero flags requires pwritev2 support on Linux 4.7 or newer."
| The operating system may set a limit (:func:`sysconf` value | ||
| ``'SC_IOV_MAX'``) on the number of buffers that can be used. | ||
|
|
||
| Availability: Linux 2.6.30 abd newer, FreeBSD 6.0 and newer, |
There was a problem hiding this comment.
should be "Availability: Linux 2.6.30, FreeBSD 6.0 and newer."
There was a problem hiding this comment.
"and newer" is commonly used in os.rst documentation (i fixed the typo)
| The operating system may set a limit (:func:`sysconf` value | ||
| ``'SC_IOV_MAX'``) on the number of buffers that can be used. | ||
|
|
||
| Combine the functionality of :func:`os.readv` and :func:`os.pread`. |
There was a problem hiding this comment.
This reads as an instruction, but it is no longer clear it is for the implementation. Maybe drop the imperative and write “This function combines the functionality . . .”. Also applies to pwritev below.
| - :data:`RWF_NOWAIT` | ||
|
|
||
| Return the total number of bytes actually read which can be less than the | ||
| total capacity of all the objects. |
There was a problem hiding this comment.
Needs a comma: “. . . number of bytes read, which can be less . . .”. Applies to readv below too.
|
|
||
| If some data was successfully read, it will return the number of bytes read. | ||
| If no bytes were read, it will return ``-1`` and set errno to | ||
| :data:`errno.EAGAIN`. |
There was a problem hiding this comment.
I’m confused. Normally you access errno as an attribute of an exception. Do the Python wrapper(s) return −1 or raise OSError/BlockingIOError?
| (sysconf() value SC_IOV_MAX) on the number of buffers that can be used. | ||
| :func:`~os.pwritev` writes the contents of each object to the file descriptor | ||
| and returns the total number of bytes written. | ||
| Write the *buffers* contents to file descriptor *fd* at a offset *offset*, |
There was a problem hiding this comment.
a → an or the, or even better “at offset offset” would work I think.
|
I abandon my change. Feel free to reuse my change to create a new PR if you want to enhance the documentation. |
https://bugs.python.org/issue31368