ModelOriented / DALEXtra

Extensions for the DALEX package
https://ModelOriented.github.io/DALEXtra/
66 stars 10 forks source link

Possible to move reticulate to Suggests? #87

Closed juliasilge closed 1 year ago

juliasilge commented 1 year ago

Hello! ๐Ÿ‘‹ Thanks so much for all your great work on this package.

I wanted to ask if you all would be open to moving reticulate to Suggests. This is motivated in particular by how Posit Connect sets up environments for content that requires reticulate (installs Python and tried to create an environment), but I think this will come into play on other deployment targets as well. The idea here is that if you are using tidymodels, you don't need Python and reticulate installed, but if you are using a model like keras, you already have Python and reticulate from dealing with the model itself.

I took a look at how you are using reticulate, and I think you could move reticulate to Suggests if you wrapped calls in rlang::is_installed() or used a base R equivalent. For example, your .onLoad() could become:

# Check if conda is present. If not, warning will be raised.

.onAttach <- function(libname, pkgname) {
  if (rlang::is_installed("reticulate")) {
      is_conda <- try(reticulate::conda_binary(), silent = TRUE)
      if(inherits(is_conda, "try-error")) {
        packageStartupMessage("Anaconda not found on your computer. Conda related functionality such as create_env.R and condaenv and yml parameters from explain_scikitlearn will not be available")
      }
  }
}

What would you think about a change like this?

maksymiuks commented 1 year ago

Hi @juliasilge

First of all thanks for your appreciation of our work :)

Honestly, I was thinking about it before and I'm glad you've opened this issue prompting me to rethink this design. Over they years this package has evolved and I agree that keeping reticulate as a hard dependency does not have much sense nowadays. Therefore I'll be happy to make this change. Will ping you once it it done :)

maksymiuks commented 1 year ago

Hi @juliasilge

It took a while but DALEXtra version with reticulate moved to suggests has been pushed to CRAN last night alongside some refactor :) Let me know if I can help you with anything else!

juliasilge commented 1 year ago

This is such great news @maksymiuks! ๐Ÿ™Œ Thank you so much for working on this; it will make deploying explainers for tidymodels much easier. I'll let you know if we run into any other challenges along these lines.