SystemsBioinformatics / parcr

Construct parser combinators in R
Other
3 stars 0 forks source link

Error in older R version inspite of successful installation #10

Closed NikolaVeber closed 8 months ago

NikolaVeber commented 8 months ago

I have just stumbled upon this package: AWESOME!

After what appeared to be (to my surprise) successful installation, one of the first examples failed:

> library(parcr)
> version
platform       x86_64-pc-linux-gnu         
arch           x86_64                      
os             linux-gnu                   
system         x86_64, linux-gnu           
status                                     
major          3                           
minor          6.3                         
year           2020                        
month          02                          
day            29                          
svn rev        77875                       
language       R                           
version.string R version 3.6.3 (2020-02-29)
nickname       Holding the Windsock    

> literal('abc')(c('abc','def'))
Error in isa(x, "list"): could not find function "isa"
Traceback:

1. literal("abc")(c("abc", "def"))
2. succeed(l)(r)
3. ensure.list(left)

Now I dont have a good excuse for not updating the runtime - but it might be useful for installation to fail.

Anyhow, now I have a good motivation to update R :)

douwe commented 8 months ago

It's a bit difficult to determine when a function was introduced in R, but it seems base::isa() was introduced only in R version 4.1. Hence, I will update the requirements in the DESCRIPTION file to R (>= 4.1). The current requirement R (>= 2.10) was too optimistic.

NikolaVeber commented 8 months ago

Wow, that was quick! :) Thanks!

Not sure if it's worth the effort, but here there is a suggestion to use is() instead, which seems to be older: https://stackoverflow.com/questions/73146762/how-to-solve-the-following-error-running-rmcorr-in-r-error-in-isaparticipant

NikolaVeber commented 8 months ago

I believe I managed to do this properly... :

$ R -e "devtools::document(roclets = c('rd', 'collate', 'namespace')); devtools::build(); devtools::check()"
....
── R CMD check results ─────────────────────────────────── parcr 0.5.0.9000 ────
Duration: 17.8s

❯ checking data for ASCII and uncompressed saves ... OK
   WARNING
  ‘qpdf’ is needed for checks on size reduction of PDFs

❯ checking re-building of vignette outputs ... WARNING
  Error(s) in re-building vignettes:
    ...
  --- re-building ‘Creating_parser_combinators.Rmd’ using rmarkdown
  Quitting from lines 13-17 (Creating_parser_combinators.Rmd)
  Error: processing vignette 'Creating_parser_combinators.Rmd' failed with diagnostics:
  The function xfun::isFALSE() will be deprecated in the future. Please consider using base::isFALSE(x) or identical(x, FALSE) instead.
  --- failed re-building ‘Creating_parser_combinators.Rmd’

  --- re-building ‘Design_details.Rmd’ using rmarkdown
  Quitting from lines 13-17 (Design_details.Rmd)
  Error: processing vignette 'Design_details.Rmd' failed with diagnostics:
  The function xfun::isFALSE() will be deprecated in the future. Please consider using base::isFALSE(x) or identical(x, FALSE) instead.
  --- failed re-building ‘Design_details.Rmd’

  SUMMARY: processing the following files failed:
    ‘Creating_parser_combinators.Rmd’ ‘Design_details.Rmd’

  Error: Vignette re-building failed.
  Execution halted

❯ checking R code for possible problems ... NOTE
  ensure.list: no visible global function definition for ‘is’
  Undefined global functions or variables:
    is
  Consider adding
    importFrom("methods", "is")
  to your NAMESPACE file (and ensure that your DESCRIPTION Imports field
  contains 'methods').

0 errors ✔ | 2 warnings ✖ | 1 note ✖
Error: R CMD check found WARNINGs
Execution halted

The only change I made was isa -> is, where the other changes come from, I am not sure (I guess build process):


$ git diff
diff --git a/DESCRIPTION b/DESCRIPTION
index 73302ed..8d22a94 100644
--- a/DESCRIPTION
+++ b/DESCRIPTION
@@ -24,7 +24,7 @@ Suggests:
 Config/testthat/edition: 3
 VignetteBuilder: knitr
 Depends:
-    R (>= 4.1)
+    R (>= 3.6)
 LazyData: true
 URL: https://github.com/SystemsBioinformatics/parcr
 BugReports: https://github.com/SystemsBioinformatics/parcr/issues
diff --git a/R/utils.R b/R/utils.R
index 856ee1a..cec1020 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -20,7 +20,7 @@ is_empty_atom <- function(l) {
 ensure.list <- function(x)  {
   if (is_empty_atom(x)) list()
   else {
-    if (!isa(x, "list")) list(x) else x
+    if (!is(x, "list")) list(x) else x
     # if (!methods::is(x,'list')) list(x) else x
   }
 }
diff --git a/man/grapes-then-grapes.Rd b/man/grapes-then-grapes.Rd
index 3953c82..a0ac558 100644
--- a/man/grapes-then-grapes.Rd
+++ b/man/grapes-then-grapes.Rd
@@ -7,7 +7,9 @@
 p1 \%then\% p2
 }
 \arguments{
-\item{p1, p2}{two parsers.}
+\item{p1}{two parsers.}
+
+\item{p2}{two parsers.}
 }
 \value{
 A parser.
diff --git a/man/grapes-xthen-grapes.Rd b/man/grapes-xthen-grapes.Rd
index 40837fc..7315a1c 100644
--- a/man/grapes-xthen-grapes.Rd
+++ b/man/grapes-xthen-grapes.Rd
@@ -10,7 +10,9 @@ p1 \%xthen\% p2
 p1 \%thenx\% p2
 }
 \arguments{
-\item{p1, p2}{two parsers.}
+\item{p1}{two parsers.}
+
+\item{p2}{two parsers.}
 }
 \value{
 A parser.

After installation from local source, tested in the same environment as stated above:

> library(parcr)
> version
platform       x86_64-pc-linux-gnu         
arch           x86_64                      
os             linux-gnu                   
system         x86_64, linux-gnu           
status                                     
major          3                           
minor          6.3                         
year           2020                        
month          02                          
day            29                          
svn rev        77875                       
language       R                           
version.string R version 3.6.3 (2020-02-29)
nickname       Holding the Windsock        

> literal('abc')(c('abc','def'))
$L
'abc'
$R
'def'
douwe commented 8 months ago

I realize now why I replaced is() by isa(). I used it originally but commented it out in the ensure.list() function (file utils.R). The reason is that it created a dependency on the methods package. That's what the note is about. In principle, creating this dependency would not be a problem.

What worries me more is that the vignettes are not built under R 3.6. This would mean that if we would want to build the package under 3.6 as well that the building process of the vignettes would also have to be tweaked?

douwe commented 8 months ago

Also, not sure whether you really get the desired output. My output of your example is.

> literal('abc')(c('abc','def'))
$L
$L[[1]]
[1] "abc"

$R
[1] "def"

You see that "abc" is enclosed in the list L[[1]]. Or is that just a matter of different print methods for list objects between these R-versions?

NikolaVeber commented 8 months ago

Please don't waste the time, I tried it out more or less for fun, and to be able to procrastinate on updating a bit longer :) I believe the the difference in output lies on me executing the examples in Jupyter Notebook. Just tried it out in R console:

[Previously saved workspace restored]

> library(parcr)
> literal('abc')(c('abc','def'))
$L
$L[[1]]
[1] "abc"

$R
[1] "def"

>
NikolaVeber commented 8 months ago

Issue is fixed by bumping the min version of the package. If you need to use it with older versions, consider the workaround above. Beware, even though it appears to work fine, it has not been thoroughly tested.