[GitHub] [arrow] romainfrancois commented on pull request #8650: ARROW-10530: [R] Use Converter API to convert SEXP to Array/ChunkedArray

2021-01-09 Thread GitBox 


romainfrancois commented on pull request #8650:
URL: https://github.com/apache/arrow/pull/8650#issuecomment-756833600


   Now pulled in the commit from #8886 so that this uses `Converter::Extend()` 
instead of the R specific previous `AppendRange()`. 
   
   I believe that for the chunker class to be useful in this case, we would 
need to act on that comment: 
   
   ```
   // we could get bit smarter here since the whole batch of appendable values
   // will be rejected if a capacity error is raised
   ```  
   



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [arrow] romainfrancois commented on pull request #8650: ARROW-10530: [R] Use Converter API to convert SEXP to Array/ChunkedArray

2021-01-08 Thread GitBox 


romainfrancois commented on pull request #8650:
URL: https://github.com/apache/arrow/pull/8650#issuecomment-756833600


   Now pulled in the commit from #8886 so that this uses `Converter::Extend()` 
instead of the R specific previous `AppendRange()`. 
   
   I believe that for the chunker class to be useful in this case, we would 
need to act on that comment: 
   
   ```
   // we could get bit smarter here since the whole batch of appendable values
   // will be rejected if a capacity error is raised
   ```  
   



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [arrow] romainfrancois commented on pull request #8650: ARROW-10530: [R] Use Converter API to convert SEXP to Array/ChunkedArray

2021-01-06 Thread GitBox 


romainfrancois commented on pull request #8650:
URL: https://github.com/apache/arrow/pull/8650#issuecomment-755350170


   At the point now where I dropped the artificial `Rscalar` class and all goes 
through `AppendRange()` which is only slightly different from @kszucs take on 
#8886. 
   
   Mostly because `AppendRange()` takes start and size when IIUC `Extend()` 
assumes `start = 0`. 
   
   I could "easily" align to what #8886 does, because at the moment I'm only 
ever using `start = 0` anyway, but the logic behind having a `start` was to 
facilitate one R vector contributing to two different chunks in the chunker ... 



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [arrow] romainfrancois commented on pull request #8650: ARROW-10530: [R] Use Converter API to convert SEXP to Array/ChunkedArray

2020-12-08 Thread GitBox 


romainfrancois commented on pull request #8650:
URL: https://github.com/apache/arrow/pull/8650#issuecomment-740504045


   I believe `AppendMultiple()` is what I would be looking for. It would e.g. 
solve my dilemma about converting data frames to struct types ...
   
   I had missed the more efficient python paths, having a look. 



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [arrow] romainfrancois commented on pull request #8650: ARROW-10530: [R] Use Converter API to convert SEXP to Array/ChunkedArray

2020-11-27 Thread GitBox 


romainfrancois commented on pull request #8650:
URL: https://github.com/apache/arrow/pull/8650#issuecomment-734843309


   ping @kszucs @bkietz I don't necessarily need a full review at this point, 
as this is far from done and will need further changes, but perhaps you can 
have a look and let me know if this goes in the right direction ? cc 
@nealrichardson 



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [arrow] romainfrancois commented on pull request #8650: ARROW-10530: [R] Use Converter API to convert SEXP to Array/ChunkedArray

2020-11-27 Thread GitBox 


romainfrancois commented on pull request #8650:
URL: https://github.com/apache/arrow/pull/8650#issuecomment-734746906


   This is still quite wip, as I navigate how the various `Converter` work 
together. 
   
   It all feels a bit weird to operate in terms of single values, i.e. via the 
`Append()` methods, since R does not have such concept, this has to resort to 
many casting. 
   
   It feels particularly odd with the struct converter (converting from a data 
frame). As of right now, this is bypassing `Append()` and instead uses a 
`Visit()` method that appends several (column by column). 
   
   Would it make sense to have some sort of vectorised approach ?
   
   Should this instead figure out a way to treat data frames that the struct 
array gets built row by row instead of column by column ?



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [arrow] romainfrancois commented on pull request #8650: ARROW-10530: [R] Use Converter API to convert SEXP to Array/ChunkedArray

2020-11-13 Thread GitBox 


romainfrancois commented on pull request #8650:
URL: https://github.com/apache/arrow/pull/8650#issuecomment-726789546


   ``` r
   library(arrow, warn.conflicts = FALSE)
   
   arrow:::vec_to_arrow(1:2, int32())
   #> Array
   #> 
   #> [
   #>   1,
   #>   2
   #> ]
   arrow:::vec_to_arrow(c(1,2), float64())
   #> Array
   #> 
   #> [
   #>   1,
   #>   2
   #> ]
   ```
   
   Created on 2020-11-13 by the [reprex 
package](https://reprex.tidyverse.org) (v0.3.0.9001)



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [arrow] romainfrancois commented on pull request #8650: ARROW-10530: [R] Use Converter API to convert SEXP to Array/ChunkedArray

2020-11-13 Thread GitBox 


romainfrancois commented on pull request #8650:
URL: https://github.com/apache/arrow/pull/8650#issuecomment-726775206


   Thanks @kszucs for the direct help. This is very far from done, but it's a 
start, and perhaps we can resume the conversation here. 
   
   AFAIK, There is no R equivalent to `PyObject*`, so I'm trying here to mimic 
that with this: 
   
   ```cpp
   struct RObject {
 RVectorType rtype;
 void* data;
 bool null;
   };
   ```
   
   This will evolve (perhaps into some sort of hierarchy) I'm sure as other 
types join the implementation (for now this only handles going from an R 
integer vector (INTSXP) to an `int32` Array. 
   



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org