Dear Martin,
thanks a lot for looking into this. Of course you were right that the
fix was not complete - I apologize for not having tested what I believed
to be the solution.
My comments on the S4 classes seemed to stem from a misunderstanding on
my side. I now believe to understand that S4 classes that inherit from R
base object types might dispatch Ops for the same object types.
If the base object value of such S4 classes is unset and therefore
empty, this empty value will be passed on to e.g. Ops.data.frame where
it would trigger the same issue as e.g. logical(0).
setClass("MyClass", slots = list(x="numeric", label="character"),
contains = "numeric")
a = new("MyClass", x=3, label="FOO")
a@.Data
> logical(0)
a == data.frame(a=1:3)
# error
I understand that this is all as expected and the error should most
likely disappear with the fix you submitted for other 0-extent cases.
Thanks again and best regards,
Hilmar
Am 18/09/2019 um 11:29 schrieb Martin Maechler:
Martin Maechler
on Wed, 18 Sep 2019 10:35:42 +0200 writes:
>>>>> Hilmar Berger
>>>>> on Sat, 14 Sep 2019 13:31:27 +0200 writes:
>> Dear all,
>> I did some more tests regarding the == operator in Ops.data.frame
(see
>> below). All tests done in R 3.6.1 (x86_64-w64-mingw32).
>> I find that errors are thrown also when comparing a zero length
>> data.frame to atomic objects with length>0 which should be a valid
case
>> according to the documentation. This can be traced to a check in the
>> last line of Ops.data.frame which tests for the presence of an empty
>> result value (i.e. list() ) but does not handle a list of empty
values
>> (i.e. list(logical(0))) which in fact is generated in those cases.
>> There is a simple fix (see also below).
> I'm pretty sure what you write above is wrong: For some reason
> you must have changed more in your own version of Ops.data.frame :
> Because there's a line
> value <- unlist(value, ...)
> there, value is *not* list(logical(0)) there, but rather logical(0)
> and then indeed, your proposed line change (at the end of Ops.data.frame)
> has no effect for the examples you give.
On the other hand, there *is* a simple "fix" at the end of
Ops.data.frame() which makes all your examples "work" (i.e. not
give an error), namely
----------------------------------------------------------------------
@@ -1685,7 +1684,7 @@
else { ## 'Logic' ("&","|") and 'Compare' ("==",">","<","!=","<=",">=")
:
value <- unlist(value, recursive = FALSE, use.names = FALSE)
matrix(if(is.null(value)) logical() else value,
- nrow = nr, dimnames = list(rn,cn))
+ nrow = nr, ncol = length(cn), dimnames = list(rn,cn))
}
----------------------------------------------------------------------
i.e., explicitly specifying 'ncol' compatibly with the column names.
However, I guess that this change would *not* signal errors
where it *should* and so am *not* (yet?) proposing to "do" it.
Another remark, on S4 which you've raised several times:
As you may know that the 'Matrix' package (part of every
"regular" R installation) uses S4 "everywhere" and it does
define many methods for its Matrix classes, all in source file Matrix/R/Ops.R
the development version (in svn / subversion) being online on R-forge here:
https://r-forge.r-project.org/scm/viewvc.php/pkg/Matrix/R/Ops.R?view=markup&root=matrix
and "of course", there we define S4 group methods for Ops all
the time, and (almost) never S3 ones...
[[but I hope you don't want to start combining data frames
with Matrix package matrices, now !]]
Martin Maechler
ETH Zurich and R Core Team
______________________________________________
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
