Skip to content
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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework R code separation #32

Open
romainfrancois opened this issue Nov 14, 2023 · 0 comments · May be fixed by #81
Open

Rework R code separation #32

romainfrancois opened this issue Nov 14, 2023 · 0 comments · May be fixed by #81

Comments

@romainfrancois
Copy link
Contributor

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 romainfrancois linked a pull request May 13, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant