xeus-r icon indicating copy to clipboard operation
xeus-r copied to clipboard

Rework R code separation

Open romainfrancois opened this issue 1 year ago • 0 comments

Currently the supporting R code is loaded, in interpreter::configure_impl() from a directory based on where xr is:

void interpreter::configure_impl()
{
    SEXP sym_Sys_which = Rf_install("Sys.which");
    SEXP sym_dirname = Rf_install("dirname");
    SEXP str_xr = Rf_mkString("xr");
    SEXP call_Sys_which = PROTECT(Rf_lang2(sym_Sys_which, str_xr));
    SEXP call = PROTECT(Rf_lang2(sym_dirname, call_Sys_which));
    SEXP dir_xr = Rf_eval(call, R_GlobalEnv);
    
    std::stringstream ss;
    ss << CHAR(STRING_ELT(dir_xr, 0)) << "/../share/jupyter/kernels/xr/resources/setup.R";
    SEXP setup_R_code_path = PROTECT(Rf_mkString(ss.str().c_str()));

    SEXP sym_source = Rf_install("source");
    SEXP call_source = PROTECT(Rf_lang2(sym_source, setup_R_code_path));
    Rf_eval(call_source, R_GlobalEnv);

    UNPROTECT(4);

    r::invoke_xeusr_fn("configure");
}

That's a lot of hoops to call source('setup.R') that itself source() the other files, which gives a cheap version of a package.

We should probably :

  • move most of the R code in a proper R package, which essentially is a simplified IRkernel i.e. IRkernel minus the transport/zmq code.
  • only keep here the code that .Call()s back into the C++, i.e. the routines.cpp file. Those are systematic, so could definitely be generated, this does not necessarily need to be an .R file. The decor 📦 could help.

We could take inspiration from what IRdisplay does with options, e.g.

clear_output <- function(wait = TRUE) {
    getOption('jupyter.clear_output_func')(wait)
}

The package would have functions such as execute() , complete() etc ... and the C++ code would:

  • define/register the .Call() compatible C++ functions
  • define the generated R wrappers
  • set the associated options

The win is that the package does not have to deal with C++ code, the option serves as a sort of pimpl.

romainfrancois avatar Nov 14 '23 09:11 romainfrancois