bpo-12743: Delete obsolete comment from marshalling docs #8457
Conversation
Doc/c-api/marshal.rst
Outdated
|
|
||
| .. c:function:: long PyMarshal_ReadLongFromFile(FILE *file) | ||
|
|
||
| Return a C :c:type:`long` from the data stream in a :c:type:`FILE\*` opened | ||
| for reading. Only a 32-bit value can be read in using this function, | ||
| regardless of the native size of :c:type:`long`. | ||
|
|
||
| On error, raise an exception and return ``-1``. | ||
| On error, sets the appropriate exception (:exc:`EOFError` or |
There was a problem hiding this comment.
ValueError can't be raised in the current code. But it can raise MemoryError. It would be nice to make it raising also OSError and KeyboardInterrupt if appropriate.
There was a problem hiding this comment.
Oops, I definitely missed MemoryError (thanks!), but isn't it possible to hit the following branch?
if (read != n) {
if (!PyErr_Occurred()) {
if (read > n)
PyErr_Format(PyExc_ValueError,
"read() returned too much data: "
"%zd bytes requested, %zd returned",
n, read);
else
PyErr_SetString(PyExc_EOFError,
"EOF read where not expected");
}
return NULL;
}It would be nice to make it raising also OSError and KeyboardInterrupt if appropriate.
Is it OK to leave that to another issue? (or is there an already open issue?)
There was a problem hiding this comment.
I definitely missed MemoryError
It is missed also in lists of exceptions for PyMarshal_ReadObjectFromFile() etc.
but isn't it possible to hit the following branch?
It looks to me that this branch can be hit only when read from Python file-like object with broken readinto(). PyMarshal_ReadLongFromFile() reads from the C file, and fread() should return <= n, unless libc is is broken.
Is it OK to leave that to another issue? (or is there an already open issue?)
I'm working on this. It will be another issue.
I have doubts that it is worth to specify full lists of exceptions. They almost always will be incomplete.
There was a problem hiding this comment.
Ok, I will revert to "On error, raise an exception and return -1." in PyMarshal_ReadLongFromFile and PyMarshal_ReadShortFromFile docs.
Do you want me to remove the list of exceptions from the remaining functions?
There was a problem hiding this comment.
I have no opinion. Providing a full list of exception is unusual, but in this case it may be possible. Look at the history of this documentation, maybe there were special reasons for listing exceptions.
There was a problem hiding this comment.
Unfortunately, there is not much in the commit logs. The docs for marshal functions were added in berkerpeksag@0fae49f and they didn't change much since 2001.
I think I'm going to keep EOFError and add a general note about MemoryError.
|
See also the documentation and docstrings for |
Do you want me to mention |
|
I don't think it is worth to mention MemoryError. It can be raised by virtually any Python code, as well as KeyboardInterrupt. Reading from the Python file can raise any exception (more likely OSError), so that |
Ok, you have a point. Removed note about |
|
|
||
|
|
||
| .. c:function:: PyObject* PyMarshal_ReadObjectFromFile(FILE *file) | ||
|
|
||
| Return a Python object from the data stream in a :c:type:`FILE\*` opened for | ||
| reading. | ||
|
|
||
| On error, sets the appropriate exception (:exc:`EOFError` or | ||
| :exc:`TypeError`) and returns *NULL*. | ||
| On error, sets the appropriate exception (:exc:`EOFError`, :exc:`ValueError` |
There was a problem hiding this comment.
See also two functions below.
|
Thanks @berkerpeksag for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7. |
Also, update the list of exceptions that may raised by PyMarshal_* functions. We usually don't document exceptions raised by a function, but in this case most of them were already documented in C API and standard library documentation. (cherry picked from commit defcffd) Co-authored-by: Berker Peksag <[email protected]>
|
GH-8489 is a backport of this pull request to the 3.7 branch. |
|
GH-8490 is a backport of this pull request to the 3.6 branch. |
Also, update the list of exceptions that may raised by PyMarshal_* functions. We usually don't document exceptions raised by a function, but in this case most of them were already documented in C API and standard library documentation. (cherry picked from commit defcffd) Co-authored-by: Berker Peksag <[email protected]>
Also, update the list of exceptions that may raised by PyMarshal_* functions. We usually don't document exceptions raised by a function, but in this case most of them were already documented in C API and standard library documentation. (cherry picked from commit defcffd) Co-authored-by: Berker Peksag <[email protected]>
Also, update the list of exceptions that may raised by PyMarshal_* functions. We usually don't document exceptions raised by a function, but in this case most of them were already documented in C API and standard library documentation. (cherry picked from commit defcffd) Co-authored-by: Berker Peksag <[email protected]>
https://bugs.python.org/issue12743