-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: Float16 cast #4180
New 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
feat: Float16 cast #4180
Conversation
tustvold
left a comment
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.
Some of the overflow behaviour seems unexpected, I could be mistaken but I thought that f16 had a similar range to f32, just lower precision?
arrow-cast/src/cast.rs
Outdated
| let f16_array: ArrayRef = Arc::new(Float16Array::from(f16_values)); | ||
|
|
||
| let f64_expected = vec![ | ||
| "-inf", "-inf", "-32768.0", "-128.0", "0.0", "255.0", "inf", "inf", "inf", |
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.
-inf seems off here, unless the original value was f16::NEG_INFINITY ?
| ]; | ||
| let f16_array: ArrayRef = Arc::new(Float16Array::from(f16_values)); | ||
|
|
||
| let f64_expected = vec!["-inf", "-32768.0", "-128.0", "0.0", "255.0", "inf"]; |
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 didn't find the way in the package half how to replace inf with `null. Is this a serious problem?
|
@tustvold Sorry for the long delay. I left a few questions on this PR. Can you pay attention to them if you have time. |
|
So, what about this PR @tustvold. |
Which issue does this PR close?
Rationale for this change
Support Float16Type in
cast.What changes are included in this PR?
Are there any user-facing changes?
Yes