I'd like a code review of my pull request:
https://github.com/matplotlib/matplotlib/pull/5106
The request is meant to fix a crashing bug:
https://github.com/matplotlib/matplotlib/issues/5105
I'll try to explain what changes I'm making in the code. Since I'm not
the original author of the code, all claims about the intent of the code
are just my interpretation.
The major change is to numpy_cpp.h, the set method of the array_view
class. This method is supposed to set the fields of the array_view
object to match the array passed in. The method can return 0 for
failure, or 1 for success. In case of failure, it should free all memory
that was allocated. If it replaces the dynamically-allocated member
m_arr, the former value needs to have its reference count decremented.
The array_view class is templated on the number of dimensions ND, so
e.g. a two-dimensional view can only have a two-dimensional buffer.
Before this patch, the set method treats zero-dimensional arrays and
arrays whose first dimension is zero as compatible with any number of
dimensions, but it leaves the m_shape member NULL, which causes the
crash. I think PyArray_DIMS(m_arr) returns NULL for zero-dimensional
arrays. The zeros array that is used instead is just a constant array of
zeros.
It is not enough to just return failure in that case, since there are
many cases in the code where an empty array is assigned to a view.
Instead, I change the logic as follows:
1. if ND is zero and the number of dimensions in the assigned
buffer is zero, that counts as a success
2. if the first dimension of the assigned buffer is zero, that
counts as a success too, so you can assign an empty buffer
that doesn't have the correct number of dimensions
3. otherwise, if the number of dimensions fails to match, that's
an error.
A smaller change in _path_wrapper.cpp adds a try-catch form around code
that uses array_view so that we get more useful error messages at Python
level. The code is almost the same as a block above it, and many people
consider using exceptions for control flow a bad thing. I wanted to make
a minimal change to the code, and this preserves the semantics while
improving the error message.
···
--
Jouni K. Sepp?nen
http://www.iki.fi/jks