BUG: only tell/seek np.fromfile when buffered #6330
Conversation
|
Wouldn't it make more sense to duck-type this, i.e., only check whether the file is unbuffered when |
|
tell() doesn't fail for buffered files, the Python-side buffering just |
|
Ok. How about that? I can squash the commits into 1 if needed. |
| /* File object instances of RawIOBase are unbuffered */ | ||
| io_raw = PyObject_GetAttrString(io, "RawIOBase"); | ||
| if (io_raw == NULL) { | ||
| return NULL; |
There was a problem hiding this comment.
This is not really my forte at all, but shouldn't io be decref'd here?
|
|
|
@cjermain - The fix you did is what I had in mind; I was worried one might be left with a hanging reference to |
|
I've rebased this as a single commit. Anything else needed? |
| } | ||
| unbuf = PyObject_IsInstance(file, io_raw); | ||
| Py_DECREF(io_raw); | ||
| if (unbuf == 1) { // Unbuffered |
There was a problem hiding this comment.
No C++ comments. Put on next line with /*...*/
|
@cjermain Can you think of a way to test this? |
|
I've fixed the C styling and squashed the changes. I think the best way to test it is to construct an unbuffered FileIO object and then modify the seek/tell methods to raise an error. Then run numpy.fromfile on it. |
|
@cjermain Great. The proper place for a test looks to be |
| /* The io module is needed to determine if buffering is used */ | ||
| io = PyImport_ImportModule("io"); | ||
| if (io == NULL) { | ||
| return NULL; |
There was a problem hiding this comment.
Here and below, shouldn't the file handle be closed before returning NULL?
|
Looks like you merged master somewhere down the line, hence endolith's commit. I suspect that will go away if you update your master from upstream and rebase this branch on it. |
|
Ok, I've rebased on the master which fixed the problem. |
BUG: only tell/seek np.fromfile when buffered
|
Merged. Thanks @cjermain . |
This fixes bug #6246 in Python 3, by using the
iomodule to check if the file object is a child of theRawIOBase, which are always unbuffered. Since it is not buffered, there is no reason to call tell or seek, which can fail on some legitimate file objects (e.g. wking/pycomedi#3).