R devel - Aug 2018 - ROBUSTNESS: x || y and x && y to give warning/error if length(x) != 1 or length(y) != 1

>>>>> Joris Meys 
>>>>>     on Thu, 30 Aug 2018 14:48:01 +0200 writes:
    > On Thu, Aug 30, 2018 at 2:09 PM D?nes T?th
    > <toth.denes at kogentum.hu> wrote:
    >> Note that `||` and `&&` have never been symmetric:
    >> 
    >> TRUE || stop() # returns TRUE stop() || TRUE # returns an
    >> error
    >> 
    >> 
    > Fair point. So the suggestion would be to check whether x
    > is of length 1 and whether y is of length 1 only when
    > needed. I.e.

    > c(TRUE,FALSE) || TRUE

    > would give an error and

    > TRUE || c(TRUE, FALSE)

    > would pass.

    > Thought about it a bit more, and I can't come up with a
    > use case where the first line must pass. So if the short
    > circuiting remains and the extra check only gives a small
    > performance penalty, adding the error could indeed make
    > some bugs more obvious.

I agree "in theory".
Thank you, Henrik, for bringing it up!

In practice I think we should start having a warning signalled.
I have checked the source code in the mean time, and the check
is really very cheap
{ because it can/should be done after checking isNumber(): so
  then we know we have an atomic and can use XLENGTH() }


The 0-length case I don't think we should change as I do find
NA (is logical!) to be an appropriate logical answer.

Martin Maechler
ETH Zurich and R Core team.

    > Cheers Joris

    > -- 
    > Joris Meys Statistical consultant

On Thu, Aug 30, 2018 at 5:58 PM Martin Maechler <maechler at
stat.math.ethz.ch>
wrote:
>
> I agree "in theory".
> Thank you, Henrik, for bringing it up!
>
> In practice I think we should start having a warning signalled.
>
I agree. I wouldn't know who would count on the automatic selection of the
first value, but better safe than sorry.

> I have checked the source code in the mean time, and the check
> is really very cheap
> { because it can/should be done after checking isNumber(): so
>   then we know we have an atomic and can use XLENGTH() }
>
>
That was my idea as well after going through the source code. I didn't want
to state it as I don't know enough of the code base and couldn't see if
there were complications I missed. Thank you for confirming!

Cheers
Joris
-- 
Joris Meys
Statistical consultant

Department of Data Analysis and Mathematical Modelling
Ghent University
Coupure Links 653, B-9000 Gent (Belgium)
<https://maps.google.com/?q=Coupure+links+653,%C2%A0B-9000+Gent,%C2%A0Belgium&entry=gmail&source=g>

-----------
Biowiskundedagen 2017-2018
http://www.biowiskundedagen.ugent.be/

-------------------------------
Disclaimer : http://helpdesk.ugent.be/e-maildisclaimer.php

	[[alternative HTML version deleted]]

On Thu, Aug 30, 2018 at 10:58 AM Martin Maechler
<maechler at stat.math.ethz.ch> wrote:
>
> >>>>> Joris Meys
> >>>>>     on Thu, 30 Aug 2018 14:48:01 +0200 writes:
>
>     > On Thu, Aug 30, 2018 at 2:09 PM D?nes T?th
>     > <toth.denes at kogentum.hu> wrote:
>     >> Note that `||` and `&&` have never been symmetric:
>     >>
>     >> TRUE || stop() # returns TRUE stop() || TRUE # returns an
>     >> error
>     >>
>     >>
>     > Fair point. So the suggestion would be to check whether x
>     > is of length 1 and whether y is of length 1 only when
>     > needed. I.e.
>
>     > c(TRUE,FALSE) || TRUE
>
>     > would give an error and
>
>     > TRUE || c(TRUE, FALSE)
>
>     > would pass.
>
>     > Thought about it a bit more, and I can't come up with a
>     > use case where the first line must pass. So if the short
>     > circuiting remains and the extra check only gives a small
>     > performance penalty, adding the error could indeed make
>     > some bugs more obvious.
>
> I agree "in theory".
> Thank you, Henrik, for bringing it up!
>
> In practice I think we should start having a warning signalled.
> I have checked the source code in the mean time, and the check
> is really very cheap
> { because it can/should be done after checking isNumber(): so
>   then we know we have an atomic and can use XLENGTH() }
>
>
> The 0-length case I don't think we should change as I do find
> NA (is logical!) to be an appropriate logical answer.
Can you explain your reasoning a bit more here? I'd like to understand
the general principle, because from my perspective it's more
parsimonious to say that the inputs to || and && must be length 1,
rather than to say that inputs could be length 0 or length 1, and in
the length 0 case they are replaced with NA.

Hadley

-- 
http://hadley.nz

?On 30/08/2018, 20:15, "R-devel on behalf of Hadley Wickham"
<r-devel-bounces at r-project.org on behalf of h.wickham at gmail.com>
wrote:

    On Thu, Aug 30, 2018 at 10:58 AM Martin Maechler
    <maechler at stat.math.ethz.ch> wrote:
    >
    > >>>>> Joris Meys
    > >>>>>     on Thu, 30 Aug 2018 14:48:01 +0200 writes:
    >
    >     > On Thu, Aug 30, 2018 at 2:09 PM D?nes T?th
    >     > <toth.denes at kogentum.hu> wrote:
    >     >> Note that `||` and `&&` have never been symmetric:
    >     >>
    >     >> TRUE || stop() # returns TRUE stop() || TRUE # returns an
    >     >> error
    >     >>
    >     >>
    >     > Fair point. So the suggestion would be to check whether x
    >     > is of length 1 and whether y is of length 1 only when
    >     > needed. I.e.
    >
    >     > c(TRUE,FALSE) || TRUE
    >
    >     > would give an error and
    >
    >     > TRUE || c(TRUE, FALSE)
    >
    >     > would pass.
    >
    >     > Thought about it a bit more, and I can't come up with a
    >     > use case where the first line must pass. So if the short
    >     > circuiting remains and the extra check only gives a small
    >     > performance penalty, adding the error could indeed make
    >     > some bugs more obvious.
    >
    > I agree "in theory".
    > Thank you, Henrik, for bringing it up!
    >
    > In practice I think we should start having a warning signalled.
    > I have checked the source code in the mean time, and the check
    > is really very cheap
    > { because it can/should be done after checking isNumber(): so
    >   then we know we have an atomic and can use XLENGTH() }
    >
    >
    > The 0-length case I don't think we should change as I do find
    > NA (is logical!) to be an appropriate logical answer.
    
    Can you explain your reasoning a bit more here? I'd like to understand
    the general principle, because from my perspective it's more
    parsimonious to say that the inputs to || and && must be length 1,
    rather than to say that inputs could be length 0 or length 1, and in
    the length 0 case they are replaced with NA.
    
    Hadley
    
I would say the value NA would cause warnings later on, that are easy to track
down, so a return of NA is far less likely to cause problems than an unintended
TRUE or FALSE. And I guess there would be some code reliant on 'logical(0)
|| TRUE' returning TRUE, that wouldn't necessarily be a mistake.

But I think it's hard to predict how exactly people are using functions. I
personally can't imagine a situation where I'd use || or &&
outside an if-statement, so I'd rather have the current behaviour, because
I'm not sure if I'm reliant on logical(0) || TRUE  somewhere in my code
(even though that would be ugly code, it's not wrong per se)
But I could always rewrite it, so I believe it's more a question of how much
would have to be rewritten. Maybe implement it first in devel, to see how many
people would complain?

Emil Bode