[Rd] readBin should check that its endian argument is a legal value
I think it would be helpful if readBin checked that its endian argument is a legal value. Why? I was reviewing some of our code and noticed that the author had readBin(..., endian="network") and never having heard of "network", I looked at the man page for readBin, and it hadn't heard of "network" either. Not good. I then looked at the R code for readBin, which has this line: swap <- endian != .Platform$endian and swap is passed into the .Internal(readBin). Further use of Google revealed that "network" is a known endian in the universe, and our code was working by essentially a lucky chance that the data was "big" and our current machines are "little". Really not good. I don't know enough about endian stuff to know if it makes sense that "network" should be one of the choices for endian for readBin (which from the documentation currently are: "big", "little" or "swap"), but in my opinion R should have failed with the choice of "network" in our code in the current version of R. I did eventually find an aside in the R Data Import/Export document that "network" is "big" so I will patch our code to be legal. Of course some code may depend on this undocumented behavior, but I would guess that in the vast majority of cases an illegal value really is a mistake. Jen > sessionInfo() R version 3.6.1 (2019-07-05) Platform: x86_64-pc-linux-gnu (64-bit) Running under: Ubuntu 18.04.3 LTS Matrix products: default BLAS: /home/mbr/r-project/R-3.6.1/lib/libRblas.so LAPACK: /home/mbr/r-project/R-3.6.1/lib/libRlapack.so locale: [1] LC_CTYPE=en_US.UTF-8 LC_NUMERIC=C [3] LC_TIME=en_US.UTF-8LC_COLLATE=en_US.UTF-8 [5] LC_MONETARY=en_US.UTF-8LC_MESSAGES=en_US.UTF-8 [7] LC_PAPER=en_US.UTF-8 LC_NAME=C [9] LC_ADDRESS=C LC_TELEPHONE=C [11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C attached base packages: [1] stats graphics grDevices utils datasets methods base loaded via a namespace (and not attached): [1] compiler_3.6.1 [[alternative HTML version deleted]] __ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel
Re: [Rd] Suggestion: Make _R_CHECK_LENGTH_1_LOGIC2_=warn the default for R 3.6.2
On Mon, Nov 18, 2019 at 6:55 AM Tomas Kalibera wrote: > > On 11/18/19 3:44 PM, Henrik Bengtsson wrote: > > .On Mon, Nov 18, 2019 at 12:35 AM Martin Maechler > > wrote: > >>> Henrik Bengtsson > >>> on Sun, 17 Nov 2019 20:42:32 -0800 writes: > >> > Hi, > >> > >> > I'm not sure where we are in getting CRAN packages getting their > >> > _R_CHECK_LENGTH_1_LOGIC2_=true bugs fixed (*), but maybe it'd help > >> to > >> > make _R_CHECK_LENGTH_1_LOGIC2_=warn the new default in the upcoming > >> R > >> > 3.6.2? > >> > >> Interesting, I had similar thoughts the last few days > >> ... but not for R 3.6.x --- I think it's out of the question to > >> do that for a "minor" update --- but for R 4.0.0. > >> > >> > Warnings of type: > >> > >> > $ R --vanilla > >> >> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "warn") > >> >> c(TRUE, FALSE) && TRUE > >> > [1] TRUE > >> > Warning message: > >> > In c(TRUE, FALSE) && TRUE : 'length(x) = 2 > 1' in coercion to > >> 'logical(1)' > >> > >> > could help encourage more package maintainers to fix these bugs > >> > sooner. Enabling this warning by default is in line with what the > >> > current default behavior for _R_CHECK_LENGTH_1_CONDITION_ bugs: > >> > >> >> if (c(TRUE, FALSE)) 42 > >> > [1] 42 > >> > Warning message: > >> > In if (c(TRUE, FALSE)) 42 : > >> > the condition has length > 1 and only the first element will be used > >> > >> > which has been the case for many years. > >> > >> > Hopefully in a not too far future, we get to a point where we can > >> have > >> > _R_CHECK_LENGTH_1_LOGIC2_=true and _R_CHECK_LENGTH_1_CONDITION_=true > >> > as the new defaults. > >> > >> Exactly. My own thoughts (mentioned above) were actually more about > >> the *other* _R_CHECK_LENGTH_1_* variable, i.e., > >> _R_CHECK_LENGTH_1_CONDITION_ > >> and that as I have only noticed these days, when considering > >> the exact impact of the upcoming > >> > >> class() |--> c("matrix", "array") > >> > >> change --- has actually *not* been part of the '--as-cran' > >> checks, nor (AFAIK, but I don't really know) of extra CRAN > >> incoming checks. > >> > >> I'm proposing to add _R_CHECK_LENGTH_1_CONDITION_=true > >> to the --as-cran checks ASAP (within weeks) > > To do that, I think we would need: > > > > _R_CHECK_LENGTH_1_CONDITION_=package:_R_CHECK_PACKAGE_NAME_,abort,verbose > > > > such that the check only applies to the code of the package being R > > CMD check:ed and not to code elsewhere. If not, there will be lots of > > confusing "false-positives" and extra work for CRAN. Unfortunately, > > the (great) "package:_R_CHECK_PACKAGE_NAME_" mechanism is not > > implemented for _R_CHECK_LENGTH_1_CONDITION_. > > The mechanism should be the same for both variables (see R Internals). > It is implemented using a single function in R-devel. Oh, I am glad I was wrong about this. This is great news. Then, it is possible to enable this check by having --as-cran set: _R_CHECK_LENGTH_1_CONDITION_=package:_R_CHECK_PACKAGE_NAME_,abort,verbose As Martin says, I think this would be great to add. R CMD check --cran already does: _R_CHECK_LENGTH_1_LOGIC2_=package:_R_CHECK_PACKAGE_NAME_,abort,verbose For those who still follow, the main point of my original post had nothing to do with 'R CMD check'. Instead, I wanted base R to default to: _R_CHECK_LENGTH_1_LOGIC2_=warn such that end users (and developers) will get run-time warnings about this type of bug. Without "warn", there's a great risk these bugs will not be fixed in a very long time. Also, using "warn" will go undetected by 'R CMD check' (as now). And 'R CMD check --as-cran' will still detect them (as now). /Henrik > > Tomas > > > > >> and indeed, I > >> think we should consider to have *both* _R_CHECK_LENGTH_1_* = true > >> become the default in R 4.0.0. > > That would be great, but I'm a bit skeptical at the same time - I > > think R 4.0.0 might be too optimistic but I won't complain if that > > would be goal. I suspect there are still lots of CRAN packages that > > would break this way overnight which would cause a major headache for > > everything and much work for the CRAN Team. This is why I wrote "I'm > > not sure where we are in getting CRAN packages getting their > > _R_CHECK_LENGTH_1_LOGIC2_=true bugs fixed"; I'm not aware of anyone > > having checked the CRAN status for this bug. > > > > /Henrik > > > >> Martin Maechler > >> ETH Zurich and R Core > >> > >> > >> > (*) I understand that CRAN incoming checks with > >> > > >> _R_CHECK_LENGTH_1_LOGIC2_=package:_R_CHECK_PACKAGE_NAME_,abort,verbose, > >> > so all packages submitted to CRAN have to pass this check. I don't > >> > think Bioconductor checks for these yet, but I could be wrong. > >> > >> > /Henrik > >> > >> > __ > >> > R-devel@r-project.org mailin
Re: [Rd] Suggestion: Make _R_CHECK_LENGTH_1_LOGIC2_=warn the default for R 3.6.2
On 11/18/19 3:44 PM, Henrik Bengtsson wrote: .On Mon, Nov 18, 2019 at 12:35 AM Martin Maechler wrote: Henrik Bengtsson on Sun, 17 Nov 2019 20:42:32 -0800 writes: > Hi, > I'm not sure where we are in getting CRAN packages getting their > _R_CHECK_LENGTH_1_LOGIC2_=true bugs fixed (*), but maybe it'd help to > make _R_CHECK_LENGTH_1_LOGIC2_=warn the new default in the upcoming R > 3.6.2? Interesting, I had similar thoughts the last few days ... but not for R 3.6.x --- I think it's out of the question to do that for a "minor" update --- but for R 4.0.0. > Warnings of type: > $ R --vanilla >> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "warn") >> c(TRUE, FALSE) && TRUE > [1] TRUE > Warning message: > In c(TRUE, FALSE) && TRUE : 'length(x) = 2 > 1' in coercion to 'logical(1)' > could help encourage more package maintainers to fix these bugs > sooner. Enabling this warning by default is in line with what the > current default behavior for _R_CHECK_LENGTH_1_CONDITION_ bugs: >> if (c(TRUE, FALSE)) 42 > [1] 42 > Warning message: > In if (c(TRUE, FALSE)) 42 : > the condition has length > 1 and only the first element will be used > which has been the case for many years. > Hopefully in a not too far future, we get to a point where we can have > _R_CHECK_LENGTH_1_LOGIC2_=true and _R_CHECK_LENGTH_1_CONDITION_=true > as the new defaults. Exactly. My own thoughts (mentioned above) were actually more about the *other* _R_CHECK_LENGTH_1_* variable, i.e., _R_CHECK_LENGTH_1_CONDITION_ and that as I have only noticed these days, when considering the exact impact of the upcoming class() |--> c("matrix", "array") change --- has actually *not* been part of the '--as-cran' checks, nor (AFAIK, but I don't really know) of extra CRAN incoming checks. I'm proposing to add _R_CHECK_LENGTH_1_CONDITION_=true to the --as-cran checks ASAP (within weeks) To do that, I think we would need: _R_CHECK_LENGTH_1_CONDITION_=package:_R_CHECK_PACKAGE_NAME_,abort,verbose such that the check only applies to the code of the package being R CMD check:ed and not to code elsewhere. If not, there will be lots of confusing "false-positives" and extra work for CRAN. Unfortunately, the (great) "package:_R_CHECK_PACKAGE_NAME_" mechanism is not implemented for _R_CHECK_LENGTH_1_CONDITION_. The mechanism should be the same for both variables (see R Internals). It is implemented using a single function in R-devel. Tomas and indeed, I think we should consider to have *both* _R_CHECK_LENGTH_1_* = true become the default in R 4.0.0. That would be great, but I'm a bit skeptical at the same time - I think R 4.0.0 might be too optimistic but I won't complain if that would be goal. I suspect there are still lots of CRAN packages that would break this way overnight which would cause a major headache for everything and much work for the CRAN Team. This is why I wrote "I'm not sure where we are in getting CRAN packages getting their _R_CHECK_LENGTH_1_LOGIC2_=true bugs fixed"; I'm not aware of anyone having checked the CRAN status for this bug. /Henrik Martin Maechler ETH Zurich and R Core > (*) I understand that CRAN incoming checks with > _R_CHECK_LENGTH_1_LOGIC2_=package:_R_CHECK_PACKAGE_NAME_,abort,verbose, > so all packages submitted to CRAN have to pass this check. I don't > think Bioconductor checks for these yet, but I could be wrong. > /Henrik > __ > R-devel@r-project.org mailing list > https://stat.ethz.ch/mailman/listinfo/r-devel __ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel __ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel __ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel
Re: [Rd] Suggestion: Make _R_CHECK_LENGTH_1_LOGIC2_=warn the default for R 3.6.2
.On Mon, Nov 18, 2019 at 12:35 AM Martin Maechler wrote: > > > Henrik Bengtsson > > on Sun, 17 Nov 2019 20:42:32 -0800 writes: > > > Hi, > > > I'm not sure where we are in getting CRAN packages getting their > > _R_CHECK_LENGTH_1_LOGIC2_=true bugs fixed (*), but maybe it'd help to > > make _R_CHECK_LENGTH_1_LOGIC2_=warn the new default in the upcoming R > > 3.6.2? > > Interesting, I had similar thoughts the last few days > ... but not for R 3.6.x --- I think it's out of the question to > do that for a "minor" update --- but for R 4.0.0. > > > Warnings of type: > > > $ R --vanilla > >> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "warn") > >> c(TRUE, FALSE) && TRUE > > [1] TRUE > > Warning message: > > In c(TRUE, FALSE) && TRUE : 'length(x) = 2 > 1' in coercion to > 'logical(1)' > > > could help encourage more package maintainers to fix these bugs > > sooner. Enabling this warning by default is in line with what the > > current default behavior for _R_CHECK_LENGTH_1_CONDITION_ bugs: > > >> if (c(TRUE, FALSE)) 42 > > [1] 42 > > Warning message: > > In if (c(TRUE, FALSE)) 42 : > > the condition has length > 1 and only the first element will be used > > > which has been the case for many years. > > > Hopefully in a not too far future, we get to a point where we can have > > _R_CHECK_LENGTH_1_LOGIC2_=true and _R_CHECK_LENGTH_1_CONDITION_=true > > as the new defaults. > > Exactly. My own thoughts (mentioned above) were actually more about > the *other* _R_CHECK_LENGTH_1_* variable, i.e., _R_CHECK_LENGTH_1_CONDITION_ > and that as I have only noticed these days, when considering > the exact impact of the upcoming > > class() |--> c("matrix", "array") > > change --- has actually *not* been part of the '--as-cran' > checks, nor (AFAIK, but I don't really know) of extra CRAN > incoming checks. > > I'm proposing to add _R_CHECK_LENGTH_1_CONDITION_=true > to the --as-cran checks ASAP (within weeks) To do that, I think we would need: _R_CHECK_LENGTH_1_CONDITION_=package:_R_CHECK_PACKAGE_NAME_,abort,verbose such that the check only applies to the code of the package being R CMD check:ed and not to code elsewhere. If not, there will be lots of confusing "false-positives" and extra work for CRAN. Unfortunately, the (great) "package:_R_CHECK_PACKAGE_NAME_" mechanism is not implemented for _R_CHECK_LENGTH_1_CONDITION_. > and indeed, I > think we should consider to have *both* _R_CHECK_LENGTH_1_* = true > become the default in R 4.0.0. That would be great, but I'm a bit skeptical at the same time - I think R 4.0.0 might be too optimistic but I won't complain if that would be goal. I suspect there are still lots of CRAN packages that would break this way overnight which would cause a major headache for everything and much work for the CRAN Team. This is why I wrote "I'm not sure where we are in getting CRAN packages getting their _R_CHECK_LENGTH_1_LOGIC2_=true bugs fixed"; I'm not aware of anyone having checked the CRAN status for this bug. /Henrik > > Martin Maechler > ETH Zurich and R Core > > > > (*) I understand that CRAN incoming checks with > > _R_CHECK_LENGTH_1_LOGIC2_=package:_R_CHECK_PACKAGE_NAME_,abort,verbose, > > so all packages submitted to CRAN have to pass this check. I don't > > think Bioconductor checks for these yet, but I could be wrong. > > > /Henrik > > > __ > > R-devel@r-project.org mailing list > > https://stat.ethz.ch/mailman/listinfo/r-devel > > __ > R-devel@r-project.org mailing list > https://stat.ethz.ch/mailman/listinfo/r-devel __ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel
Re: [Rd] BUG?: A copy of base::`+` (primitive) is not a clone but a "pointer"
On 11/18/19 10:45 AM, Martin Maechler wrote: Tomas Kalibera on Mon, 18 Nov 2019 09:36:14 +0100 writes: > On 11/18/19 9:18 AM, Martin Maechler wrote: >>> Henrik Bengtsson >>> on Sun, 17 Nov 2019 14:31:07 -0800 writes: >> > $ R --vanilla R version 3.6.1 (2019-07-05) -- "Action of >> > the Toes" Copyright (C) 2019 The R Foundation for >> > Statistical Computing Platform: x86_64-pc-linux-gnu >> > (64-bit) ... >> >> >> str(base::`+`) >> > function (e1, e2) >> >> >> plus <- structure(base::`+`, class = "plus") str(plus) >> > function (e1, e2) - attr(*, "class")= chr "plus" >> >> > ## Hmm ... >> >> str(base::`+`) >> > function (e1, e2) - attr(*, "class")= chr "plus" >> >> >> class(base::`+`) <- NULL str(base::`+`) >> > function (e1, e2) >> >> > ## Hmm ... >> >> str(plus) >> > function (e1, e2) >> >> > Even without assigning to `plus`, you get this behavior: >> >> > $ R --vanilla >> >> structure(base::`+`, class = "plus") >> > function (e1, e2) .Primitive("+") attr(,"class") [1] >> > "plus" >> >> > # Hmm... >> >> str(base::`+`) >> > function (e1, e2) - attr(*, "class")= chr "plus" >> >> > Looks to be the case for common (all?) .Primitive >> > functions. >> >> No need for 'base::' (who would be crazy enough to redefine `+`?) >> nor str() actually: >> >> attr(`+`, "class") <- NULL # (reset) >> `+` >> structure(`+`, class = "plus") >> `+` >> >> is clearly convincing and minimal >> >>> attr(`+`, "class") <- NULL >>> `+` >> function (e1, e2) .Primitive("+") >>> structure(`+`, class = "plus") >> function (e1, e2) .Primitive("+") >> attr(,"class") >> [1] "plus" >>> `+` >> function (e1, e2) .Primitive("+") >> attr(,"class") >> [1] "plus" >> - >> >> > Is this expected? >> >> no. (at least not by 99.999% of R users) >> >> >> > Should I report this one to Bugzilla? >> yes, please. >> >> > /Henrik > A shorter example is >> p1 <- .Primitive('+') ; p2 <- p1 ; attr(p1, "myattr") <- 1 ; p2 > function (e1, e2) .Primitive("+") > attr(,"myattr") > [1] 1 beautiful ; thank you, Tomas ! > Builtins have referential semantics in R (like e.g. environments, but > also some other types). > Tomas [aarh.. I knew it ... but am showing my age: I had forgotten about it.] I forget (and don't have time just now to find out) where we have documented it; it may be good to document also in official user exposed places such say the ?.Primitive help page. I think internal objects such as builtins should never be modified by user code in the first place. We had to add detection for modifications of NIL and symbols in the past, maybe we will have to do it for builtins as well. Environments can be used in user code as objects with referential semantics, e.g. to keep mutable state. Tomas Martin __ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel
Re: [Rd] Inappropriate class(o)[!inherits(o,"AsIs")] in get_all_vars
> Martin Maechler > on Mon, 18 Nov 2019 12:15:38 +0100 writes: > suharto anggono--- via R-devel > on Sun, 17 Nov 2019 10:34:31 + writes: >> SVN revision 77401 changes >> x[isM] <- lapply(x[isM], function(o) `class<-`(o, class(o)[class(o) != "AsIs"])) >> to >> x[isM] <- lapply(x[isM], function(o) `class<-`(o, class(o)[!inherits(o,"AsIs")])) >> in function 'get_all_vars' in src/library/stats/R/models.R in R devel. >> The change is inappropriate. >> class(o)[class(o) != "AsIs"] removes "AsIs" from class(o), giving class(o) without "AsIs". >> On the other hand, inherits(o,"AsIs") is just a single logical value. If "AsIs" is in class(o), inherits(o,"AsIs") is TRUE. In that case, by recycling of logical index, class(o)[!inherits(o,"AsIs")] removes all elements of class(o), giving character(0). > Thank you, Suharto ! > You are obviously right, and I'm a bit embarrassed by my > overzealousness to follow my own recommendations in the R Blog > http://bit.ly/R_blog_class_think_2x > {*wrongly*: The recommendation was to "think again" ...} > It's a "shame" that the wrong code did not trigger any checks, > so if anybody has time... I'd be grateful for such a regression > check. Once I started thinking, it was easy to modify the previous reg.check to trigger in the case of the erronous r77401. Fixed now in 77435. Martin __ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel
Re: [Rd] Inappropriate class(o)[!inherits(o,"AsIs")] in get_all_vars
> suharto anggono--- via R-devel > on Sun, 17 Nov 2019 10:34:31 + writes: > SVN revision 77401 changes > x[isM] <- lapply(x[isM], function(o) `class<-`(o, class(o)[class(o) != "AsIs"])) > to > x[isM] <- lapply(x[isM], function(o) `class<-`(o, class(o)[!inherits(o,"AsIs")])) > in function 'get_all_vars' in src/library/stats/R/models.R in R devel. > The change is inappropriate. > class(o)[class(o) != "AsIs"] removes "AsIs" from class(o), giving class(o) without "AsIs". > On the other hand, inherits(o,"AsIs") is just a single logical value. If "AsIs" is in class(o), inherits(o,"AsIs") is TRUE. In that case, by recycling of logical index, class(o)[!inherits(o,"AsIs")] removes all elements of class(o), giving character(0). Thank you, Suharto ! You are obviously right, and I'm a bit embarrassed by my overzealousness to follow my own recommendations in the R Blog http://bit.ly/R_blog_class_think_2x {*wrongly*: The recommendation was to "think again" ...} It's a "shame" that the wrong code did not trigger any checks, so if anybody has time... I'd be grateful for such a regression check. Martin __ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel
Re: [Rd] BUG?: A copy of base::`+` (primitive) is not a clone but a "pointer"
> Tomas Kalibera > on Mon, 18 Nov 2019 09:36:14 +0100 writes: > On 11/18/19 9:18 AM, Martin Maechler wrote: >>> Henrik Bengtsson >>> on Sun, 17 Nov 2019 14:31:07 -0800 writes: >> > $ R --vanilla R version 3.6.1 (2019-07-05) -- "Action of >> > the Toes" Copyright (C) 2019 The R Foundation for >> > Statistical Computing Platform: x86_64-pc-linux-gnu >> > (64-bit) ... >> >> >> str(base::`+`) >> > function (e1, e2) >> >> >> plus <- structure(base::`+`, class = "plus") str(plus) >> > function (e1, e2) - attr(*, "class")= chr "plus" >> >> > ## Hmm ... >> >> str(base::`+`) >> > function (e1, e2) - attr(*, "class")= chr "plus" >> >> >> class(base::`+`) <- NULL str(base::`+`) >> > function (e1, e2) >> >> > ## Hmm ... >> >> str(plus) >> > function (e1, e2) >> >> > Even without assigning to `plus`, you get this behavior: >> >> > $ R --vanilla >> >> structure(base::`+`, class = "plus") >> > function (e1, e2) .Primitive("+") attr(,"class") [1] >> > "plus" >> >> > # Hmm... >> >> str(base::`+`) >> > function (e1, e2) - attr(*, "class")= chr "plus" >> >> > Looks to be the case for common (all?) .Primitive >> > functions. >> >> No need for 'base::' (who would be crazy enough to redefine `+`?) >> nor str() actually: >> >> attr(`+`, "class") <- NULL # (reset) >> `+` >> structure(`+`, class = "plus") >> `+` >> >> is clearly convincing and minimal >> >>> attr(`+`, "class") <- NULL >>> `+` >> function (e1, e2) .Primitive("+") >>> structure(`+`, class = "plus") >> function (e1, e2) .Primitive("+") >> attr(,"class") >> [1] "plus" >>> `+` >> function (e1, e2) .Primitive("+") >> attr(,"class") >> [1] "plus" >> - >> >> > Is this expected? >> >> no. (at least not by 99.999% of R users) >> >> >> > Should I report this one to Bugzilla? >> yes, please. >> >> > /Henrik > A shorter example is >> p1 <- .Primitive('+') ; p2 <- p1 ; attr(p1, "myattr") <- 1 ; p2 > function (e1, e2) .Primitive("+") > attr(,"myattr") > [1] 1 beautiful ; thank you, Tomas ! > Builtins have referential semantics in R (like e.g. environments, but > also some other types). > Tomas [aarh.. I knew it ... but am showing my age: I had forgotten about it.] I forget (and don't have time just now to find out) where we have documented it; it may be good to document also in official user exposed places such say the ?.Primitive help page. Martin __ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel
Re: [Rd] BUG?: A copy of base::`+` (primitive) is not a clone but a "pointer"
On 11/18/19 9:18 AM, Martin Maechler wrote: Henrik Bengtsson on Sun, 17 Nov 2019 14:31:07 -0800 writes: > $ R --vanilla R version 3.6.1 (2019-07-05) -- "Action of > the Toes" Copyright (C) 2019 The R Foundation for > Statistical Computing Platform: x86_64-pc-linux-gnu > (64-bit) ... >> str(base::`+`) > function (e1, e2) >> plus <- structure(base::`+`, class = "plus") str(plus) > function (e1, e2) - attr(*, "class")= chr "plus" > ## Hmm ... >> str(base::`+`) > function (e1, e2) - attr(*, "class")= chr "plus" >> class(base::`+`) <- NULL str(base::`+`) > function (e1, e2) > ## Hmm ... >> str(plus) > function (e1, e2) > Even without assigning to `plus`, you get this behavior: > $ R --vanilla >> structure(base::`+`, class = "plus") > function (e1, e2) .Primitive("+") attr(,"class") [1] > "plus" > # Hmm... >> str(base::`+`) > function (e1, e2) - attr(*, "class")= chr "plus" > Looks to be the case for common (all?) .Primitive > functions. No need for 'base::' (who would be crazy enough to redefine `+`?) nor str() actually: attr(`+`, "class") <- NULL # (reset) `+` structure(`+`, class = "plus") `+` is clearly convincing and minimal attr(`+`, "class") <- NULL `+` function (e1, e2) .Primitive("+") structure(`+`, class = "plus") function (e1, e2) .Primitive("+") attr(,"class") [1] "plus" `+` function (e1, e2) .Primitive("+") attr(,"class") [1] "plus" - > Is this expected? no. (at least not by 99.999% of R users) > Should I report this one to Bugzilla? yes, please. > /Henrik A shorter example is > p1 <- .Primitive('+') ; p2 <- p1 ; attr(p1, "myattr") <- 1 ; p2 function (e1, e2) .Primitive("+") attr(,"myattr") [1] 1 Builtins have referential semantics in R (like e.g. environments, but also some other types). Tomas __ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel __ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel
Re: [Rd] Suggestion: Make _R_CHECK_LENGTH_1_LOGIC2_=warn the default for R 3.6.2
> Henrik Bengtsson > on Sun, 17 Nov 2019 20:42:32 -0800 writes: > Hi, > I'm not sure where we are in getting CRAN packages getting their > _R_CHECK_LENGTH_1_LOGIC2_=true bugs fixed (*), but maybe it'd help to > make _R_CHECK_LENGTH_1_LOGIC2_=warn the new default in the upcoming R > 3.6.2? Interesting, I had similar thoughts the last few days ... but not for R 3.6.x --- I think it's out of the question to do that for a "minor" update --- but for R 4.0.0. > Warnings of type: > $ R --vanilla >> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "warn") >> c(TRUE, FALSE) && TRUE > [1] TRUE > Warning message: > In c(TRUE, FALSE) && TRUE : 'length(x) = 2 > 1' in coercion to 'logical(1)' > could help encourage more package maintainers to fix these bugs > sooner. Enabling this warning by default is in line with what the > current default behavior for _R_CHECK_LENGTH_1_CONDITION_ bugs: >> if (c(TRUE, FALSE)) 42 > [1] 42 > Warning message: > In if (c(TRUE, FALSE)) 42 : > the condition has length > 1 and only the first element will be used > which has been the case for many years. > Hopefully in a not too far future, we get to a point where we can have > _R_CHECK_LENGTH_1_LOGIC2_=true and _R_CHECK_LENGTH_1_CONDITION_=true > as the new defaults. Exactly. My own thoughts (mentioned above) were actually more about the *other* _R_CHECK_LENGTH_1_* variable, i.e., _R_CHECK_LENGTH_1_CONDITION_ and that as I have only noticed these days, when considering the exact impact of the upcoming class() |--> c("matrix", "array") change --- has actually *not* been part of the '--as-cran' checks, nor (AFAIK, but I don't really know) of extra CRAN incoming checks. I'm proposing to add _R_CHECK_LENGTH_1_CONDITION_=true to the --as-cran checks ASAP (within weeks) and indeed, I think we should consider to have *both* _R_CHECK_LENGTH_1_* = true become the default in R 4.0.0. Martin Maechler ETH Zurich and R Core > (*) I understand that CRAN incoming checks with > _R_CHECK_LENGTH_1_LOGIC2_=package:_R_CHECK_PACKAGE_NAME_,abort,verbose, > so all packages submitted to CRAN have to pass this check. I don't > think Bioconductor checks for these yet, but I could be wrong. > /Henrik > __ > R-devel@r-project.org mailing list > https://stat.ethz.ch/mailman/listinfo/r-devel __ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel
Re: [Rd] BUG?: A copy of base::`+` (primitive) is not a clone but a "pointer"
> Henrik Bengtsson > on Sun, 17 Nov 2019 14:31:07 -0800 writes: > $ R --vanilla R version 3.6.1 (2019-07-05) -- "Action of > the Toes" Copyright (C) 2019 The R Foundation for > Statistical Computing Platform: x86_64-pc-linux-gnu > (64-bit) ... >> str(base::`+`) > function (e1, e2) >> plus <- structure(base::`+`, class = "plus") str(plus) > function (e1, e2) - attr(*, "class")= chr "plus" > ## Hmm ... >> str(base::`+`) > function (e1, e2) - attr(*, "class")= chr "plus" >> class(base::`+`) <- NULL str(base::`+`) > function (e1, e2) > ## Hmm ... >> str(plus) > function (e1, e2) > Even without assigning to `plus`, you get this behavior: > $ R --vanilla >> structure(base::`+`, class = "plus") > function (e1, e2) .Primitive("+") attr(,"class") [1] > "plus" > # Hmm... >> str(base::`+`) > function (e1, e2) - attr(*, "class")= chr "plus" > Looks to be the case for common (all?) .Primitive > functions. No need for 'base::' (who would be crazy enough to redefine `+`?) nor str() actually: attr(`+`, "class") <- NULL # (reset) `+` structure(`+`, class = "plus") `+` is clearly convincing and minimal > attr(`+`, "class") <- NULL > `+` function (e1, e2) .Primitive("+") > structure(`+`, class = "plus") function (e1, e2) .Primitive("+") attr(,"class") [1] "plus" > `+` function (e1, e2) .Primitive("+") attr(,"class") [1] "plus" > - > Is this expected? no. (at least not by 99.999% of R users) > Should I report this one to Bugzilla? yes, please. > /Henrik __ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel