jonkeane commented on code in PR #48634:
URL: https://github.com/apache/arrow/pull/48634#discussion_r2643286779
##########
r/src/altrep.cpp:
##########
@@ -1160,20 +1168,21 @@ sexp test_arrow_altrep_is_materialized(sexp x) {
return Rf_ScalarLogical(NA_LOGICAL);
}
- sexp data_class_sym = CAR(ATTRIB(ALTREP_CLASS(x)));
- std::string class_name(CHAR(PRINTNAME(data_class_sym)));
-
int result = NA_LOGICAL;
- if (class_name == "arrow::array_dbl_vector") {
+ if (R_altrep_inherits(x,
arrow::r::altrep::AltrepVectorPrimitive<REALSXP>::class_t)) {
result =
arrow::r::altrep::AltrepVectorPrimitive<REALSXP>::IsMaterialized(x);
- } else if (class_name == "arrow::array_int_vector") {
+ } else if (R_altrep_inherits(
+ x, arrow::r::altrep::AltrepVectorPrimitive<INTSXP>::class_t))
{
result =
arrow::r::altrep::AltrepVectorPrimitive<INTSXP>::IsMaterialized(x);
- } else if (class_name == "arrow::array_string_vector") {
+ } else if (R_altrep_inherits(
+ x,
arrow::r::altrep::AltrepVectorString<arrow::StringType>::class_t)) {
result =
arrow::r::altrep::AltrepVectorString<arrow::StringType>::IsMaterialized(x);
- } else if (class_name == "arrow::array_large_string_vector") {
+ } else if (R_altrep_inherits(
+ x,
+
arrow::r::altrep::AltrepVectorString<arrow::LargeStringType>::class_t)) {
result =
arrow::r::altrep::AltrepVectorString<arrow::LargeStringType>::IsMaterialized(x);
- } else if (class_name == "arrow::array_factor") {
+ } else if (R_altrep_inherits(x, arrow::r::altrep::AltrepFactor::class_t)) {
result = arrow::r::altrep::AltrepFactor::IsMaterialized(x);
Review Comment:
I haven't pulled the code locally — but do we use `class_name` elsewhere or
was this the one place? Maybe we could get rid of `class_name` entirely if we
always do something like ^^^^?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]