nealrichardson commented on a change in pull request #8246:
URL: https://github.com/apache/arrow/pull/8246#discussion_r494391841
##########
File path: r/src/arrow_cpp11.h
##########
@@ -157,8 +157,15 @@ struct ns {
template <typename Pointer>
Pointer r6_to_pointer(SEXP self) {
- return reinterpret_cast<Pointer>(
- R_ExternalPtrAddr(Rf_findVarInFrame(self, arrow::r::symbols::xp)));
+ void* p = R_ExternalPtrAddr(Rf_findVarInFrame(self, arrow::r::symbols::xp));
+ if (p == nullptr) {
Review comment:
Ah ok, I missed that that check translated some responses into
`NULL`--though I don't think that's something we do widely, and maybe it would
be better to make that more explicit in order to simplify the rest of the
cases.
Regarding directly returning R6 objects from the cpp code, my thought had
been more like the first idea of modifying `as_sexp` to map `T` to R6 classes.
Our convention is that R6 class names are generally the bare C++ class name
(assuming namespaces), i.e. `arrow::dataset::Dataset` is `Dataset` R6 class,
with some exceptions. So we could record a map of those exceptions (e.g.
"arrow::csv::TableReader" maps to "CsvTableReader") in cpp. If I were writing
this in R, it could look something like this:
```r
get_r6_constructor <- function(cpp_class) {
if (cpp_class %in% names(special_cases)) {
r6_name <- special_cases[[cpp_class]]
} else {
r6_name <- sub("^.*::(.*)$", "\\1", cpp_class)
}
get(r6_name, asNamespace("arrow"))$new
}
```
cc @bkietz
----------------------------------------------------------------
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
