Re: all.equal() improvements (PR#8191)

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

Re: all.equal() improvements (PR#8191)

Martin Maechler
I've now finally finalized my work on a subset of Andy's
propositions, and committed it to R-devel.

The current change doesn't show in our own checks and examples,
but may well in other people's package checks.
For this reason, I've also added a line to the
'USER-VISIBLE CHANGES' part of the NEWS file.

Martin

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

Re: all.equal() improvements (PR#8191)

Martin Maechler
I'm "happy" to have found the first problem myself:

'Matrix' doesn't pass R CMD check  anymore with the change I had
committed:

Basically because of this:

  > all.equal(cbind(1:5), matrix(1:5, 5,1, dimnames=list(NULL,NULL)))
  [1] "Attributes: < Names: Lengths (1, 2) differ (string compare on first 1) >"
  [2] "Attributes: < Length mismatch: comparison on first 1 components >"      

This new behavior is "S-compatible" insofar as S-plus 6.1 also
returns non-TRUE.

Is this what we want?
{we'll see soon how many other CRAN packages are having problems for it}

In my intuition, I'd have liked all.equal()  to return TRUE for the above,
since in principle,  dimnames = NULL  or dimnames = list(NULL,NULL)
is a trivial difference.
OTOH, it will need "special case" code to assure this, and I
wonder if that's worth it.

Please comment!
Martin

>>>>> "MM" == Martin Maechler <[hidden email]>
>>>>>     on Mon,  2 Jan 2006 18:00:15 +0100 (CET) writes:

    MM> I've now finally finalized my work on a subset of Andy's
    MM> propositions, and committed it to R-devel.

    MM> The current change doesn't show in our own checks and
    MM> examples, but may well in other people's package checks.
    MM> For this reason, I've also added a line to the
    MM> 'USER-VISIBLE CHANGES' part of the NEWS file.

    MM> Martin

    MM> ______________________________________________
    MM> [hidden email] mailing list
    MM> 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: all.equal() improvements (PR#8191)

Prof Brian Ripley
Martin,

I have some tests running over CRAN now (RUnit has also failed), but have
already noticed things like

> swiss[, 1] -> x
> names(x) <- rownames(swiss)
> all.equal(x, x[1:10])
[1] "Names: Lengths (47, 10) differ (string compare on first 10)"
[2] "Numeric: lengths (47, 10) differ"

which is telling me the obvious, with the result that the reports from
e.g. rpart are cluttered to the detriment of legibility.

I think we need to think harder about what should be reported when the
objects differ in mode or length.

Brian

On Mon, 2 Jan 2006 [hidden email] wrote:

> I'm "happy" to have found the first problem myself:
>
> 'Matrix' doesn't pass R CMD check  anymore with the change I had
> committed:

I am seeing a problem in setGeneric which stops it being installed.

>
> Basically because of this:
>
>  > all.equal(cbind(1:5), matrix(1:5, 5,1, dimnames=list(NULL,NULL)))
>  [1] "Attributes: < Names: Lengths (1, 2) differ (string compare on first 1) >"
>  [2] "Attributes: < Length mismatch: comparison on first 1 components >"
>
> This new behavior is "S-compatible" insofar as S-plus 6.1 also
> returns non-TRUE.
>
> Is this what we want?
> {we'll see soon how many other CRAN packages are having problems for it}
>
> In my intuition, I'd have liked all.equal()  to return TRUE for the above,
> since in principle,  dimnames = NULL  or dimnames = list(NULL,NULL)
> is a trivial difference.
> OTOH, it will need "special case" code to assure this, and I
> wonder if that's worth it.
>
> Please comment!
> Martin
>
>>>>>> "MM" == Martin Maechler <[hidden email]>
>>>>>>     on Mon,  2 Jan 2006 18:00:15 +0100 (CET) writes:
>
>    MM> I've now finally finalized my work on a subset of Andy's
>    MM> propositions, and committed it to R-devel.
>
>    MM> The current change doesn't show in our own checks and
>    MM> examples, but may well in other people's package checks.
>    MM> For this reason, I've also added a line to the
>    MM> 'USER-VISIBLE CHANGES' part of the NEWS file.

--
Brian D. Ripley,                  [hidden email]
Professor of Applied Statistics,  http://www.stats.ox.ac.uk/~ripley/
University of Oxford,             Tel:  +44 1865 272861 (self)
1 South Parks Road,                     +44 1865 272866 (PA)
Oxford OX1 3TG, UK                Fax:  +44 1865 272595

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

Re: all.equal() improvements (PR#8191)

Martin Maechler
>>>>> "BDR" == Prof Brian Ripley <[hidden email]>
>>>>>     on Mon, 2 Jan 2006 20:39:18 +0000 (GMT) writes:

    BDR> Martin,
    BDR> I have some tests running over CRAN now (RUnit has also failed),

thank you, Brian, for the feedback

    BDR> but have  already noticed things like

    >> swiss[, 1] -> x
    >> names(x) <- rownames(swiss)
    >> all.equal(x, x[1:10])
    BDR> [1] "Names: Lengths (47, 10) differ (string compare on first 10)"
    BDR> [2] "Numeric: lengths (47, 10) differ"

    BDR> which is telling me the obvious, with the result that the reports from
    BDR> e.g. rpart are cluttered to the detriment of legibility.

    BDR> I think we need to think harder about what should be reported when the
    BDR> objects differ in mode or length.

I agree;  the above is good example.
[OTOH, I don't think the above behavior to be a complete show
 stopper; since it's somewhat close to the way S-plus does things]

    BDR> Brian

    BDR> On Mon, 2 Jan 2006 [hidden email] wrote:

    >> I'm "happy" to have found the first problem myself:
    >>
    >> 'Matrix' doesn't pass R CMD check  anymore with the change I had
    >> committed:

    BDR> I am seeing a problem in setGeneric which stops it being installed.

{ah yes;  for R-devel you need the "next" version of Matrix
 the important part of which I'll commit shortly to R-packages;  it
 will take another day before I'll upload it to CRAN}


    >> Basically because of this:
    >>
    >> > all.equal(cbind(1:5), matrix(1:5, 5,1, dimnames=list(NULL,NULL)))
    >> [1] "Attributes: < Names: Lengths (1, 2) differ (string compare on first 1) >"
    >> [2] "Attributes: < Length mismatch: comparison on first 1 components >"
    >>
    >> This new behavior is "S-compatible" insofar as S-plus 6.1 also
    >> returns non-TRUE.
    >>
    >> Is this what we want?
    >> {we'll see soon how many other CRAN packages are having problems for it}
    >>
    >> In my intuition, I'd have liked all.equal()  to return TRUE for the above,
    >> since in principle,  dimnames = NULL  or dimnames = list(NULL,NULL)
    >> is a trivial difference.
    >> OTOH, it will need "special case" code to assure this, and I
    >> wonder if that's worth it.
    >>
    >> Please comment!
    >> Martin
    >>
    >>>>>>> "MM" == Martin Maechler <[hidden email]>
    >>>>>>> on Mon,  2 Jan 2006 18:00:15 +0100 (CET) writes:
    >>
    MM> I've now finally finalized my work on a subset of Andy's
    MM> propositions, and committed it to R-devel.
    >>
    MM> The current change doesn't show in our own checks and
    MM> examples, but may well in other people's package checks.
    MM> For this reason, I've also added a line to the
    MM> 'USER-VISIBLE CHANGES' part of the NEWS file.

    BDR> --
    BDR> Brian D. Ripley,                  [hidden email]
    BDR> Professor of Applied Statistics,  http://www.stats.ox.ac.uk/~ripley/
    BDR> University of Oxford,             Tel:  +44 1865 272861 (self)
    BDR> 1 South Parks Road,                     +44 1865 272866 (PA)
    BDR> Oxford OX1 3TG, UK                Fax:  +44 1865 272595

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