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

Unable to run autotest in the "sits" package #81

Open
gilbertocamara opened this issue Jul 8, 2023 · 8 comments
Open

Unable to run autotest in the "sits" package #81

gilbertocamara opened this issue Jul 8, 2023 · 8 comments

Comments

@gilbertocamara
Copy link

As part of the pre-submission process of the package "sits" to the ROpenSci statistical review, as recommended by @maelle, I am running autotest_package . I made sure that all the examples and tests run OK. However, when running autotest_package, I am finding an error, which appears to be related to an internal check in autotest. Please see the MWE

devtools::install_github("e-sensing/sits@dev")
# All examples run OK
withr::with_envvar(new = c("SITS_RUN_EXAMPLES" = "TRUE"), devtools::run_examples())
(...) 
# autotest_package fails
withr::with_envvar(new = c("SITS_RUN_EXAMPLES" = "TRUE"), autotest::autotest_package("sits"))

Error in if (qts[[i]][1] > 0) { : missing value where TRUE/FALSE needed

Given that all examples run, could you please help find what the problem is?

@maelle
Copy link
Contributor

maelle commented Jul 11, 2023

I can reproduce the error. Now let me dig a bit more... (the maintainer is on vacation, so I'll try to step in efficiently 😁 )

@maelle
Copy link
Contributor

maelle commented Jul 11, 2023

@gilbertocamara the problem comes from the example of the documentation for sits_run_examples(). The problem needs to be fixed on our side but I also wonder why that function is exposed to users? Why export it? And why is the example an example as opposed to some text? I'm a bit confused.

@gilbertocamara
Copy link
Author

Dear @maelle Point taken! We only included the example in the documentation of sits_run_examples() because this was required by ROpenSci guidelines. I will remove the examples in sits_run_examples() and sits_run_tests(). The reason we need to expose this function is because we cannot afford to run the examples when running CRAN "checks", because they would take too long to run, and might fail because some of the cloud services may be unavailable.

I have removed the example in the documentation for sits_run_examples() and commited the changes to the "dev" version. Now, I hope autotest will run OK. I'll now run it on my side.

@maelle
Copy link
Contributor

maelle commented Jul 11, 2023

reprex for autotest bug

ex <- c("if (TRUE) {", "    message(\"blop\")", 
"} else {", "    message(\"bla\"", "    ))", "}")
autotest:::parse_expressions (ex)
#> Error in if (qts[[i]][1] > 0) {: missing value where TRUE/FALSE needed

Created on 2023-07-11 with reprex v2.0.2

@gilbertocamara
Copy link
Author

Hi @maelle, great help! I am running autotest on my side. I will keep you informed.

@maelle
Copy link
Contributor

maelle commented Jul 11, 2023

Notes on the bug.

The input of parse_expressions() is used in a loop

https://github.com/ropensci-review-tools/autotest/blob/1a4e37db462e2eaaf9834f97dfad08551151496f/R/text-parsing-fns.R#L365-366

However the input is already overwritten at the first step of the loop

x <- c (xfirst, xmid, xlast)

So at the second step, the subsetting at the beginning of the loop can't work.

@gilbertocamara
Copy link
Author

Dear @maelle, autotest now works! You can close the issue if you wish so. Thanks!!

@maelle
Copy link
Contributor

maelle commented Jul 11, 2023

Thanks! I'll keep it open because there's still a bug here but I'm thrilled to know this is no longer a blocker for sits. 😁

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

No branches or pull requests

2 participants