richardjtelford / checker

Checks R Configuration Setup Correctly Before Class
https://richardjtelford.github.io/checker/
Other
1 stars 1 forks source link

Support arbitrary user-provided checks #1

Open JsLth opened 2 weeks ago

JsLth commented 2 weeks ago

Thanks for making this package! It seems very interesting for a couple of use cases.

I am wondering if it is feasible to extend chk_make() to support arbitrary requirements. Something along the lines of:

---
R:
  recommended: 4.3.0
  minimum: 4.2.0
extra:
  check_java:
    version: 11
    condition: >
      has_java <- nzchar(Sys.which("java"))
      if (!has_java) return(FALSE)
      java_version <- system2("java", "--version")
      java_version <- paste(java_version, collapse = "\n")
      java_version <- regmatches(java_version, regexec("java ([0-9]{1,2})", java_version))[[1]][2]
      numeric_version({version}) > numeric_version(java_version)
    message_fail: >
      Java version > {version} is required to run this tutorial.
      You can download the newest Java version from https://website.org/
    message_success: >
      Java version {version} is installed.
---

Note that the condition for the check is self-provided by the user. In this case, I want to establish a check that checks if the right version of java is installed. I can imagine passing parameters using the neighboring fields and then glueing them into the condition string using glue::glue() (also note the curly braces around the version variable). Additionally, users can provide a fail or success message in a similar fashion. A chk_extra function could look something like so:

chk_extra <- function(yam) {
  lapply(names(yam), function(field) {
    x <- yam[[field]]]
    chk_sanity(
      actual = names(x),
      exptected = "condition",
      message = sprintf("Configuration '%s' does not contain a condition.", field)
    )
    cond <- glue::glue(x$condition, .envir = x)
    cond <- str2lang(cond)
    chk_arbitrary <- function() {}
    body(chk_arbitrary) <- cond

    if (!chk_arbitrary()) {
      msg <- glue::glue(x$message_fail, .envir = x)
      outcome <- chk_cat(
        message = msg,
        status = "danger"
      )
    } else {
      msg <- glue::glue(x$message_success, .envir = x)
      outcome <- chk_cat(
        message = msg,
        status = "success"
      )
    }
  })
}

Of course, the code chunk is very bare bones and I don't know if it works. It only serves to illustrate the idea. Do you think this could be a reasonable addition to the package? If so, I could prepare a pull request. I think this would greatly expand the scope of this package.

richardjtelford commented 2 weeks ago

Thank you for these ideas and your interest in my package. I have been contemplating extending the package to test for other requirements, but the idea of coding these one by one is rather daunting. The idea of making this the users responsibility to manage this is attractive.

I do have a few concerns though.

I'm happy to take advice on these issues, but would be happier to hard code the checks.

Beyond java, do you have suggestions of what other checks would be useful?

JsLth commented 2 weeks ago

I understand your concerns on security and cross-OS compatibility, although I would argue that students always take this risk when executing code given to them by an instructor. Unfortunately, I don't think it is possible to properly sanitize or control the consequences of arbitrary R code, especially given the fact that users can use base R methods like Sys.chmod() or fork bombs to perform malicious activities on students' machines.

Ultimately, I think most use cases for this idea can surely be hard-coded. Some ideas:

richardjtelford commented 1 week ago

Some of these I need for my teaching - I have big problems with locale on windows computers making it difficult to work with Norwegian letters - and I see the utility of most of the rest (not sure why the time zone check is useful). Doesn't pandoc come with quarto or rmarkdown automatically - does it need testing for separately?

I'd like to implement as much of this as possible without requiring extra dependencies. One of the design principals I had when writing the package was to use as few packages as possible as each package increases the probability of something breaking.

One possibility would be to suggest packages like stevedore rather than imported them so only student who need the test need to install it. If the package checks are run before the extra tests, this should work nicely.

Longer term, it might be necessary to look at how checker would work with positron or other IDE.

I'd also like the package to include some tests with testthat.

I'm in the middle of teaching at the moment (students used checker today) so it will take a while before I have time to look at checker again, but pull requests are welcome.