feat: astype: accept a kind of data type
#848
Conversation
rgommers
added
the
API change
There was a problem hiding this comment.
Thanks for getting the ball rolling on this @lucascolley! Some comments inline.
| For ``dtype_or_kind`` a data type, an array having the specified data type. | ||
| For ``dtype_or_kind`` a kind of data type: | ||
| - If ``x.dtype`` is already of that kind, the data type is maintained. | ||
| - Otherwise, an attempt is made to convert to the specified kind, according to the type promotion rules (see :ref:`type-promotion`). |
There was a problem hiding this comment.
Why "an attempt"? That seems ambiguous. We have to be clear about what must work. Which I think is:
- float to complex
- unsigned to signed integer
Anything else doesn't I think? There's no point allowing 'bool' I think, since there is only one boolean dtype so dtype=xp.bool will be cleaner.
For 'signed integer' and 'real floating-point'` there are also no promotion rules to follow, so they can be left out - or do you see a use case?
There was a problem hiding this comment.
I've reduced this down to just 'complex floating' (use-case: mixed float/complex to complex) and 'signed integer' (use-case: mixed signed/unsigned to signed).
I think "an attempt" would still be accurate for an implementation of this? xp.astype(some_int8_array, 'complex floating') would attempt a conversion, whose success will depend on the implementation-specific type promotion rules, right?
Unless you think that this function should always error when the type promotion is not defined by the standard?
There was a problem hiding this comment.
I think "an attempt" would still be accurate for an implementation of this?
I think you have the right idea in mind here, it's just a "language we use to specify things" thing. We specify which behavior has to be supported - 'complex floating' has type promotion rules defined in the standard, so it's expected to always work for a compliant implementation. Then, if we expect other input types to raise, then we specify that by "must raise ..." or "input type must be ...". In this case there's no reason to do that (implementors are free to suppport more types, it's just not standardized), so we then say "input type should be ...".
Your "attempt to ..." seems to be the same as "should be ...", it's just language we want to write in a uniform way.
There was a problem hiding this comment.
how about the wording now?
There was a problem hiding this comment.
Quick update: I took the liberty to update the wording. I also made the call to broaden the list of data type kinds. I think there are reasonable arguments for providing any one of the numeric data type kinds (e.g., int32 and real floating => float64, etc), and it is possible to delineate a set of clearly defined rules in terms of which data type should be returned. Leaving bool out seems somewhat arbitrary, especially when the semantics are clearly specified and all other kinds can be, IMO, reasonably provided (note: even including "numeric"; i.e., convert anything provided to me to numbers so I can compute the sum, etc).
| - If ``x.dtype`` is already of that kind, the data type must be maintained. | ||
| - Otherwise, ``x`` should be cast to a data type of that kind, according to the type promotion rules (see :ref:`type-promotion`) and the above notes. | ||
| - Kinds must be interpreted as the lowest-precision standard data type of that kind for the purposes of type promotion. For example, ``astype(x, 'complex floating')`` will return an array with the data type ``complex64`` when ``x.dtype`` is ``float32``, since ``complex64`` is the result of promoting ``float32`` with the lowest-precision standard complex data type, ``complex64``. | ||
| - Where type promotion is unspecified and thus implementation-specific, the result is also unspecified. For example, ``astype(x, 'complex floating')``, where ``x`` has data type ``int32``. |
There was a problem hiding this comment.
This is overly restrictive, and conflicts with the behavior specified above. The whole point of astype is to be able to do casts that aren't in the promotion table, like int -> float casts. In fact, I would say this whole line should be deleted as the above note already clearly talks about what is and isn't defined.
There was a problem hiding this comment.
This is overly restrictive, and conflicts with the behavior specified above. The whole point of astype is to be able to do casts that aren't in the promotion table, like int -> float casts
I think there is a misunderstanding - this point applies only in the case that a kind of data type is provided, as this is under the bullet list started on line 63.
Ralf suggested in #848 (comment) that it doesn't make sense to support kinds for which the resulting dtype would always be undefined, given that there are no type promotion rules to follow. Do you disagree?
There was a problem hiding this comment.
I agree with Aaron here. I've updated the wording accordingly. It should be possible, e.g., to be able to do astype(x, dtype="real floating") where x is int32 in order to return a float64 array. This conversion between dtype kinds is not part of type promotion rules, but has well-defined behavior, as int32 can be represented exactly in float64.
There was a problem hiding this comment.
This conversion between dtype kinds is not part of type promotion rules, but has well-defined behavior, as int32 can be represented exactly in float64.
Okay, yes, that sounds right to me now.
|
|
||
| - If ``x.dtype`` is already of that kind, the data type must be maintained. | ||
| - Otherwise, ``x`` should be cast to a data type of that kind, according to the type promotion rules (see :ref:`type-promotion`) and the above notes. | ||
| - Kinds must be interpreted as the lowest-precision standard data type of that kind for the purposes of type promotion. For example, ``astype(x, 'complex floating')`` will return an array with the data type ``complex64`` when ``x.dtype`` is ``float32``, since ``complex64`` is the result of promoting ``float32`` with the lowest-precision standard complex data type, ``complex64``. |
There was a problem hiding this comment.
The text you've added here seems to only be thinking about the case where the dtype argument is a string, but the dtype being an actual dtype object is also still supported.
There was a problem hiding this comment.
Line 62 covers the dtype case. Do you have a suggestion to make it more clear?
There was a problem hiding this comment.
@asmeurer Would you be willing to take a look at the latest text to determine whether this satisfies your concerns?
|
Gentle reminder of the 2024 milestone here! |
There was a problem hiding this comment.
just one nit, otherwise this new wording LGTM, thanks @kgryte !
| When casting a boolean input array to a real-valued data type, a value of ``True`` must cast to a real-valued number equal to ``1``, and a value of ``False`` must cast to a real-valued number equal to ``0``. | ||
| - When applying type promotion rules, the returned array must have the lowest-precision data type belonging to the specified data type kind to which the data type of ``x`` promotes (e.g., if ``x`` is ``float32`` and the data type kind is ``'complex floating'``, then the returned array must have the data type ``complex64``; if ``x`` is ``uint16`` and the data type kind is ``'signed integer'``, then the returned array must have the data type ``int32``). | ||
| - When type promotion rules are not specified between the data type of ``x`` and the specified data type kind (e.g., ``int16`` and ``'real floating'``) and there exists one or more data types belonging to the specified data kind in which the elements in ``x`` can be represented exactly (e.g., ``int32`` and ``float64``), the function must return an array having the smallest data type (in terms of range of values) capable of precisely representing the elements of ``x`` (e.g., if ``x`` is ``int16`` and the data type kind is ``'complex floating'``, then the returned array must have the data type ``complex64``; if ``x`` is `bool`` and the data type kind is ``integral``, then the returned array must have the data type ``int8``). | ||
| - When type promotion rules are not specified between the data type of ``x`` and the specified data type kind and there neither exists a data type belonging to the specified data type in which the elements of ``x`` can be represented exactly (e.g., ``uint64`` and ``'real floating'``) nor are there applicable casting rules documented below, behavior is unspecified and thus implementation-defined. |
There was a problem hiding this comment.
| - When type promotion rules are not specified between the data type of ``x`` and the specified data type kind and there neither exists a data type belonging to the specified data type in which the elements of ``x`` can be represented exactly (e.g., ``uint64`` and ``'real floating'``) nor are there applicable casting rules documented below, behavior is unspecified and thus implementation-defined. | |
| - When type promotion rules are not specified between the data type of ``x`` and there neither exists a data type belonging to the specified data type in which the elements of ``x`` can be represented exactly (e.g., ``uint64`` and ``'real floating'``) nor are there applicable casting rules documented below, behavior is unspecified and thus implementation-defined. |
This PR:
dtypefromuinttointandfloattocomplex? #859 by adding data type "kind" support inastype.