Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
feat: add support for specifying a data type "kind" in
astype
#848New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: add support for specifying a data type "kind" in
astype
#848Changes from 4 commits
2a82758
419041b
302cf1a
3186995
90aa750
41880c0
d4450b3
1b056ad
169ea5e
ecd0f59
b61122a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why "an attempt"? That seems ambiguous. We have to be clear about what must work. Which I think is:
Anything else doesn't I think? There's no point allowing
'bool'
I think, since there is only one boolean dtype sodtype=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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about the wording now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
andreal floating
=>float64
, etc), and it is possible to delineate a set of clearly defined rules in terms of which data type should be returned. Leavingbool
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).