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

Fix js lookup syntax error #1100

Merged
merged 1 commit into from
Oct 6, 2023
Merged

Fix js lookup syntax error #1100

merged 1 commit into from
Oct 6, 2023

Conversation

andrjohns
Copy link
Contributor

Summary:

The onLoad action for rstan currently uses the $validate() member function to check the stanc.js in StanHeaders, and if the check fails then loads the stanc.js from rstan. However, $validate() assumes that the input is a string of JS code, not a file path. This means that the check is always FALSE, and the rstan stanc.js is always loaded.

This PR simplifies that to just checking whether the file exists.

Copyright and Licensing

Please list the copyright holder for the work you are submitting (this will be you or your assignee, such as a university or company): Andrew Johnson

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the following licenses:

@andrjohns andrjohns merged commit cc2598a into develop Oct 6, 2023
7 checks passed
@andrjohns andrjohns deleted the js-lookup-error branch October 6, 2023 12:45
@bgoodri
Copy link
Contributor

bgoodri commented Oct 8, 2023

The mustWork = TRUE argument to system.file already requires the file to exist, but if it exists in the corrupted state, it will cause things to break. I will put this in .onLoad in the the 2.32 branch to get around it:

  stanc_js <- system.file("stanc.js", package = "StanHeaders", mustWork = TRUE)
  test <- try(stanc_ctx$source(stanc_js), silent = TRUE)
  if (inherits(test, "try-error")) {
    stanc_js <- system.file("exec", "stanc.js", package = "rstan", mustWork = TRUE)
    stanc_ctx$source(stanc_js)
  }

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 this pull request may close these issues.

2 participants