ENH: Allow creating structured void scalars by passing dtype#22316
ENH: Allow creating structured void scalars by passing dtype#22316seberg merged 2 commits intonumpy:mainfrom
Conversation
mhvk
left a comment
mhvk
left a comment
There was a problem hiding this comment.
Looks good! Only some small points. A more general one is whether keyword-only is needed here. By analogy with np.array, I think having the dtype as the second positional argument is OK.
| match="void: descr must be a `void.*int64"): | ||
| np.void(4, dtype="i8") | ||
|
|
||
| dtype = np.dtype("4i") |
There was a problem hiding this comment.
Why is this defined? I would use '4i' for clarity in the actual test.
There was a problem hiding this comment.
whoops, wanted to avoid a TypeError due to getting it wrong in the test, and then did it wrong :). Will just match the error string instead.
seberg
force-pushed
the
void-strctured-new
branch
from
37c6a54 to
61976b9
Compare
|
OK, updated the typing stubs and test (not sure that it is correct) and fixed up those things. Also allowed it to be passed positionally, it really seems unnecessary to enforce keyword here. (And added a brief release note.) I think it should be OK now modulo the typing/typing tests. |
seberg
force-pushed
the
void-strctured-new
branch
2 times, most recently
from
dbcd09f to
dca9d09
Compare
|
@BvB93 I guess it doesn't work easily to reject certain objects only if |
seberg
force-pushed
the
void-strctured-new
branch
3 times, most recently
from
7f953e7 to
44863c9
Compare
mhvk
left a comment
mhvk
left a comment
There was a problem hiding this comment.
Looks all good except possible the typing error.
Adds an optional `dtype=` kwarg to `np.void`. If given (and not None),
this kwarg effectively turns it into:
res = np.array(data, dtype=dtype)[()]
Thanks for Marten's review and Bas' help with the typing.
Reviewed-by: Marten van Kerkwijk <[email protected]>
Co-authored-by: Bas van Beek <[email protected]>
seberg
force-pushed
the
void-strctured-new
branch
from
44863c9 to
519ce63
Compare
seberg
added
the
62 - Python API
|
I think this is good to go, unless anyone thinks the API addition needs to be announced wider. |
numpy/core/_add_newdocs_scalars.py
Outdated
| be returned. | ||
| dtype : dtype, optional | ||
| If provided the dtype of the new scalar. This dtype must | ||
| be "void" dtype (i.e. a structured or unstructured |
There was a problem hiding this comment.
Can this link to the definition of void https://numpy.org/devdocs/reference/arrays.scalars.html#numpy.void?
There was a problem hiding this comment.
Linking to https://numpy.org/devdocs/user/basics.rec.html#structured-datatypes (lets see if that works), that seems useful. This is that documentation, so that would be circular ;).
| has three calling conventions: | ||
|
|
||
| 1. ``np.void(5)`` creates a ``dtype="V5"`` scalar filled with | ||
| ``\0`` bytes. The 5 can be a Python or NumPy integer. |
mattip
left a comment
mattip
left a comment
There was a problem hiding this comment.
Just some documentation nits, feel free to mark them as rejected if they don't make sense.
Co-authored-by: Matti Picus <[email protected]>
|
Thanks Matti. Tweaked the docs according to those notes and tests are passing now. So just going ahead and merging myself. |
4 participants
Adds an optional
dtype=kwarg tonp.void. If given (and not None), this kwarg effectively turns it into:The new dtype argument is keyword-only.
The whole idea behind is that if NEP 51 (scalar repr changes) flies, it would be nice if a new void representation is copy-pastable. This achieves that. So it is a small enhancement (I think), that is a stepping stone for fixing scalar representations.