Repository navigation
DOC: improvement of the documentation for gufunc. - #11177
Conversation
| Must be of length (*nin* + *nout*) \* *ntypes*, and it | ||
| contains the data-types (built-in only) that the corresponding | ||
| function in the *func* array can deal with. | ||
| Int8 of length `(nin + nout) * ntypes` It encodes the :ref:`dtype.num` |
There was a problem hiding this comment.
Int8 is only true on some platforms, where char is signed.
Might be better to refer to PyArray_Descr.type_num here, since that's the C-side name. Could refer to both.
| (built-in only) that the corresponding function in the `func` array | ||
| accepts. For a ufunc with two `ntypes`, one `nin` and one `nout` where | ||
| the first function accepts and returns `int32` and the second accepts | ||
| and returns `int64`, `types` would be `\05\05\07\07` since `int32.num` |
There was a problem hiding this comment.
Double backticks for code snippets.
Would be clearer here to say types would be ``(char[]) {5, 5, 7, 7}`` , I think - no need to reference a string literal
| accepts. For a ufunc with two `ntypes`, one `nin` and one `nout` where | ||
| the first function accepts and returns `int32` and the second accepts | ||
| and returns `int64`, `types` would be `\05\05\07\07` since `int32.num` | ||
| is 5 and `int64.num` is 7. |
There was a problem hiding this comment.
Potentially misleading - this is np.dtype(np.int64).num, not np.int64.num
|
All good points; adjusted. |
There was a problem hiding this comment.
Char array would be clearer. capitalizing Char looks weird too.
Could take advantage of C 2d arrays here, and write this as ``char[ntypes][nin+nout]`` array
There was a problem hiding this comment.
Like it in principle, but as this is not done anywhere in the code, it may lead to confusion...
There was a problem hiding this comment.
I suppose I'll have to make a patch at some point to make it so we do do that in the code.
There was a problem hiding this comment.
That would definitely make the definitions a lot clearer!
There was a problem hiding this comment.
With the 2d notation above, this becomes (char[2][2]) {{5, 5}, {7, 7}};. To break symmetry between the 2s, I'd be inclined to use a binary op as an example.
There was a problem hiding this comment.
Yes, I'll use a comparison as an example instead.
f73b118 to
8ca8b0a
Compare
There was a problem hiding this comment.
(``unsigned char``) is just wrong here. NPY_BOOL is its own type.
There was a problem hiding this comment.
Hmm, I got this from
grep npy_bool numpy/core/include/numpy/*.h
.
.
.
numpy/core/include/numpy/npy_common.h:typedef unsigned char npy_bool;
There was a problem hiding this comment.
NPY_BOOL is the enum value, which is distinct from NPY_UCHAR - even if the same underlying C type is used to represent them, at the numpy level they are different.
There was a problem hiding this comment.
Maybe the description is a bit hinging on different things. One passes to the function definition those NPY_ names, but inside the routine one would use npy_bool, npy_int32,64, etc.
There was a problem hiding this comment.
Maybe just change your description to use npy_int32 and npy_bool - then you don't need to explain what those typedefs mean. If you single-backtick them, they might link to their definitions.
There was a problem hiding this comment.
Yes, and that also shows recommended practice, so better.
There was a problem hiding this comment.
I think the npy_* are indeed linkable, though I cannot seem to build the docs - get a very helpful error message...
ValueError: The section Notes appears twice in the docstring of None in None.
Bit beyond this PR...
There was a problem hiding this comment.
@mhvk Hmm, last time I saw that error it was because masked array was modifying docstrings. I think that was fixed.
8ca8b0a to
d7f4eef
Compare
Cherry-picked part of #11132, which is nice & independent of the enhancement proposed there.