[Rd] readBin should check that its endian argument is a legal value

2019-11-18 Thread Jennifer Lyon
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

2019-11-18 Thread Henrik Bengtsson
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

2019-11-18 Thread Tomas Kalibera

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

2019-11-18 Thread Henrik Bengtsson
.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"

2019-11-18 Thread Tomas Kalibera

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

2019-11-18 Thread Martin Maechler
> 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

2019-11-18 Thread Martin Maechler
> 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"

2019-11-18 Thread Martin Maechler
> 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"

2019-11-18 Thread Tomas Kalibera

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

2019-11-18 Thread Martin Maechler
> 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"

2019-11-18 Thread Martin Maechler
> 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