Suggestion: Make _R_CHECK_LENGTH_1_LOGIC2_=warn the default for R 3.6.2

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Suggestion: Make _R_CHECK_LENGTH_1_LOGIC2_=warn the default for R 3.6.2

Henrik Bengtsson-5
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? 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.

(*) 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

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: Suggestion: Make _R_CHECK_LENGTH_1_LOGIC2_=warn the default for R 3.6.2

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(<matrix>)  |-->  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

    > ______________________________________________
    > [hidden email] mailing list
    > https://stat.ethz.ch/mailman/listinfo/r-devel

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: Suggestion: Make _R_CHECK_LENGTH_1_LOGIC2_=warn the default for R 3.6.2

Henrik Bengtsson-5
.On Mon, Nov 18, 2019 at 12:35 AM Martin Maechler
<[hidden email]> 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(<matrix>)  |-->  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
>
>     > ______________________________________________
>     > [hidden email] mailing list
>     > https://stat.ethz.ch/mailman/listinfo/r-devel
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: Suggestion: Make _R_CHECK_LENGTH_1_LOGIC2_=warn the default for R 3.6.2

Tomas Kalibera
On 11/18/19 3:44 PM, Henrik Bengtsson wrote:

> .On Mon, Nov 18, 2019 at 12:35 AM Martin Maechler
> <[hidden email]> 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(<matrix>)  |-->  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
>>
>>      > ______________________________________________
>>      > [hidden email] mailing list
>>      > https://stat.ethz.ch/mailman/listinfo/r-devel
>>
>> ______________________________________________
>> [hidden email] mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: Suggestion: Make _R_CHECK_LENGTH_1_LOGIC2_=warn the default for R 3.6.2

Henrik Bengtsson-5
On Mon, Nov 18, 2019 at 6:55 AM Tomas Kalibera <[hidden email]> wrote:

>
> On 11/18/19 3:44 PM, Henrik Bengtsson wrote:
> > .On Mon, Nov 18, 2019 at 12:35 AM Martin Maechler
> > <[hidden email]> 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(<matrix>)  |-->  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
> >>
> >>      > ______________________________________________
> >>      > [hidden email] mailing list
> >>      > https://stat.ethz.ch/mailman/listinfo/r-devel
> >>
> >> ______________________________________________
> >> [hidden email] mailing list
> >> https://stat.ethz.ch/mailman/listinfo/r-devel
> > ______________________________________________
> > [hidden email] mailing list
> > https://stat.ethz.ch/mailman/listinfo/r-devel
>
>

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel