Bioconductor / SummarizedExperiment

SummarizedExperiment container
https://bioconductor.org/packages/SummarizedExperiment
29 stars 9 forks source link

`all.equal(x, y)` vs `all(x == y)` #64

Closed LiNk-NY closed 2 years ago

LiNk-NY commented 2 years ago

Hi Hervé, @hpages

When there are missing (NA) row names, the dim names check in assays_have_expected_dimnames returns NA whereas identical returns TRUE when row names are missing in both comparisons. Perhaps it should use all.equal? It less strict than identical.

a_rownames <- c("XKRX", "XKRY2", "XKRY", NA, "XPA", "XPC", "XPNPEPL", "XPNPEP2", 
"XPNPEP3", "XPO1", "XPO4", "XPO5", "XPO6", "XPO7", "XPOT", "XPR1", 
"XRCC1", "XRCC2", "XRCC3", "XRCC4", "XRCC5")
expected_dimnames <- list(a_rownames)
all(a_rownames == expected_dimnames[[1L]])
#' [1] NA
identical(a_rownames, expected_dimnames[[1L]])
#' [1] TRUE
all.equal(a_rownames, expected_dimnames[[1L]])
#' [1] TRUE

https://github.com/Bioconductor/SummarizedExperiment/blob/dd615ee820975bfbf6fae71da29e9c7fb976fe60/R/SummarizedExperiment-class.R#L267-L274

Reproducible example:

m <- matrix(c(1L, 2L, 3L, 5L, 6L, 7L, 9L, 10L, 11L), nrow = 3, dimnames = list(c("a", "b", NA), c("a", "b", "c")))
m
#>      a b  c
#> a    1 5  9
#> b    2 6 10
#> <NA> 3 7 11
SummarizedExperiment::SummarizedExperiment(m)
#> Error in if (!ok) {: missing value where TRUE/FALSE needed

Created on 2022-05-13 by the reprex package (v2.0.1)

BiotechPedro commented 2 years ago

Wow! Thanks for the explanation! Shouldn't this be corrected?

hpages commented 2 years ago

ooh, and if the NAs are in the colnames then we run into a different problem:

> SummarizedExperiment::SummarizedExperiment(t(m))
Error in DataFrame(x = seq_len(ncol(a1)), row.names = nms) : 
  missing values in 'row.names'

Thanks for the report. Working on a fix...

hpages commented 2 years ago

Fixed (sort of) in SummarizedExperiment 1.27.2. BUT...

NAs in the dimnames of a SummarizedExperiment derivative should be avoided as they will cause problems. See IMPORTANT NOTES in commit message 71872cc03b7c0195fb80d1d09409243f049ebb3f.

Cheers, H.