knausb / vcfR

Tools to work with variant call format files
248 stars 54 forks source link

class == matrix #150

Closed knausb closed 4 years ago

knausb commented 4 years ago

Ping?

Please correct before 2020-01-10 to safely retain your package on CRAN.

Note that this will be the final reminder.

-k

Dear maintainer, Please see the problems shown on https://cran.r-project.org/web/checks/check_results_vcfR.html.

Specifically, see the problems shown for r-devel-linux-x86_64-debian-clang.

These can be reproduced by checking with --as-cran using current r-devel, which for now sets

_R_CLASS_MATRIXARRAY=true _R_CHECK_LENGTH_1CONDITION="package:_R_CHECK_PACKAGENAME,verbose"

in the check environment, to the effect that

R> class(matrix(1 : 4, 2, 2))

[1] "matrix" "array"

(and no longer just "matrix" as before), and that conditions of length greater than one in 'if' and 'while' statements executing in the package being checked give an error.

According to the R NEWS file,

For now only active when environment variable _R_CLASS_MATRIXARRAY is set to non-empty, but planned to be the new unconditional behavior when R 4.0.0 is released:

matrix objects now also inherit from class "array", namely, e.g., class(diag(1)) is c("matrix", "array") which invalidates code assuming that length(class(obj)) == 1, an incorrect assumption that is less frequently fulfilled now.

S3 methods for "array", i.e., .array(), are now also dispatched for matrix objects.

Apparently your package no longer works correctly when class(matrix(...)) gives a vector of length two and conditions of length greater than one in 'if' or 'while' give an error: please fix as necessary.

See https://developer.r-project.org/Blog/public/2019/11/09/when-you-think-class.-think-again/index.html for more information about correctly using class() in package code.

Please correct before 2019-12-18 to safely retain your package on CRAN.

Best, -k

knausb commented 4 years ago

Did some homework. The link at https://developer.r-project.org/Blog/public/2019/11/09/when-you-think-class.-think-again/index.html was very helpful. I use

if (class(obj) == "matrix")  { ..... }   # *bad* - do not copy !

to test if a function was passed a matrix. This is not recommended and needs to be changed to the following.

inherits(x, "matrix")

And we need to test this on Rdevel.

knausb commented 4 years ago

Tried to reproduce the behaviour on using rhub using the following version of R.

R Under development (unstable) (2020-01-03 r77629) -- "Unsuffered Consequences"

But was unable to reproduce the error.

knausb commented 4 years ago

Reproduced it on travis (build #1168) with:

R Under development (unstable) (2020-01-03 r77628) -- "Unsuffered Consequences"
knausb commented 4 years ago

At 609f577c0a7173950976e9557d99331346269a3d we're now building cleanly at travis ci :smile: