Support OBJSXP coming in R 4.4.0 (closes #1283) by eddelbuettel · Pull Request #1293 · RcppCore/Rcpp (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation5 Commits2 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

This addressed #1283 and the addition of OBJSXP as a new value / replacement for S4SXP to better support S7 changes. I ran a full reverse-depends check but given that the change is conditional on R 4.4.0 or later nothing much came up.

It is something we probably want to get into 1.0.12 for the next cycle. and I don't think it will do anything wrong but reviews would be much appreciated as I may still have forgotten something.

• Code compiles correctly
• R CMD check still passes all tests
• Preferably, new tests were added which fail without the change
• Document the changes by file in ChangeLog

I think we still want to return S4SXP in the appropriate cases. So following this, in case OBJSXP,

• if (IS_S4_OBJECT(x)) return "S4SXP";
• otherwise, return "OBJSXP";

EDIT: Maybe Rf_isS4 instead of IS_S4_OBJECT?

I like that a lot. So how about

#if R_Version >= R_Version(4,4,0) // replaces S4SXP in R 4.4.0 case OBJSXP: return Rf_isS4(x) ? "S4SXP" : "OBJSXP"; // cf src/main/inspect.c #else case S4SXP: return "S4SXP"; #endif

moving the switch inside the 'after R 4.4.0' conditional and returning 'S4SXP' for S4 as before and 'OBJSXP' otherwise.

Even seems to work:

library(Rcpp) M <- Matrix::rsparsematrix(2,2,0.5) cppFunction("const char* t(SEXP x) { return type2name(x); }") t(1L) [1] "INTSXP" t(M) [1] "S4SXP" getRversion() [1] ‘4.4.0’

And now I get to learn how to create an S7 object :)

Edit: No luck. All I get is CLOSXP but I chalk that up to the S7 package rather than the diff here.

Choose a reason for hiding this comment

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

LGTM!

2 participants