Skip to content
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

ARROW-3741: [R] Add support for arrow::compute::Cast to convert Arrow arrays from one type to anothe #2959

Closed
wants to merge 10 commits into from

Conversation

romainfrancois
Copy link
Contributor

@romainfrancois romainfrancois commented Nov 14, 2018

library(arrow)
a <- array(1:10, NA)
a$type()
#> arrow::Int32 
#> int32

b <- a$cast(int16())
b$type()
#> arrow::Int16 
#> int16

Created on 2018-11-14 by the reprex package (v0.2.1.9000)

@romainfrancois
Copy link
Contributor Author

romainfrancois commented Nov 14, 2018

This needs more work (more tests) and support to cast Table as well, but opening early because I'm seeing some weird stuff:

library(arrow)

a <- array(-(1:3))
a$cast(uint16())
#> Error in Array__cast(self, target_type, options): Invalid: Integer value out of bounds
a$cast(uint16())
#> Error in Array__cast(self, target_type, options): Invalid: Integer value out of bounds
a$cast(uint32())
#> arrow::Array 
#> [
#>   4294967295,
#>   4294967294,
#>   4294967293
#> ]
a$cast(uint32())$type()
#> arrow::UInt32 
#> uint32

a$cast(uint64())
#> arrow::Array 
#> [
#>   -1,
#>   -2,
#>   -3
#> ]
a$cast(uint64())$type()
#> arrow::UInt64 
#> uint64

Created on 2018-11-14 by the reprex package (v0.2.1.9000)

The first two error are what I expect, because safe is true by default in the cast options, but I'd expect the other one to also fail, and not give these results.

@romainfrancois
Copy link
Contributor Author

closes #2953

@romainfrancois romainfrancois force-pushed the ARROW-3741/Cast branch 2 times, most recently from 4d8e53c to 2ab336c Compare November 14, 2018 13:46
@wesm
Copy link
Member

wesm commented Nov 14, 2018

In the future when you see something that looks buggy, there's no need to wait to open a JIRA (or go through the mailing list to confirm a bug) =)

@wesm
Copy link
Member

wesm commented Nov 22, 2018

Is this ready for review? There's a lot of R PR's open; I would appreciate some tips about which ones need to be reviewed

@romainfrancois
Copy link
Contributor Author

There's often a rebasing needed between two squashes, probably because some files always change e.g. the generated files or the DESCRIPTION file.

I've added the labels lang-R and ready-for-review I hope it's alright. Perhaps what I can do is mark the one I'd like to be merged next as ready-for-review and the one that really still needs work as WIP

image

@wesm
Copy link
Member

wesm commented Nov 22, 2018

Thanks, "ready-for-review" works for me.

Copy link
Member

@wesm wesm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. thanks @romainfrancois!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants