const-ae / glmGamPoi

Fit Gamma-Poisson Generalized Linear Models Reliably
105 stars 14 forks source link

Error parsing "complex" contrast names #30

Closed TobiTekath closed 2 years ago

TobiTekath commented 2 years ago

Hi Constantin,

I just came across an error in the handling of contrast names in test_de(). The error appears when using a model.matrix with "complex" names (see below)

Code to reproduce:

library(glmGamPoi)
Y <- matrix(rnbinom(n = 30 * 100, mu = 4, size = 0.3), nrow = 30, ncol = 100)
annot <- data.frame(sample = sample(LETTERS[1:6], size = 100, replace = TRUE),
                    grp = sample(LETTERS[1:2], size = 100, replace = TRUE))
se <- SummarizedExperiment::SummarizedExperiment(Y, colData = annot)
mm <- model.matrix(~annot$grp)
fit <- glm_gp(se, design = mm)
test_de(fit, contrast = "annot$grpB")

Error:

 Error in array(x, c(length(x), 1L), if (!is.null(names(x))) list(names(x),  : 
  'data' must be of a vector type, was 'NULL'

I traced the Error to https://github.com/const-ae/glmGamPoi/blob/b595fd7fbdba98a89ca57dbcbb38baa151e4c767/R/parse_contrast.R#L38 which returns NULL in this context (and not the correct column name as supposed).

The error can be circumvented by defining the model.matrix more elegantly (model.matrix(~grp, annot)), but as the matrices themselves are identical (except for the column names), I still think this might be an error you want to catch.

Best, Tobi

const-ae commented 2 years ago

Hi Tobi,

thanks for the report and the detailed description which made it easy to understand what is happening. The easiest way to avoid that error is actually to use

test_de(fit, contrast = "`annot$grpB`")

Note the ticks ` around annot$grpB. This is necessary to make sure that the contrast is indeed interpretated as a name and not some R expression.

I am not sure that this is a problem that I would be able to detect in parse_contrast. However, if you have a suggestion to provide a more meaningful error message, I would be happy to hear :)

TobiTekath commented 2 years ago

Hi Constantin,

thank you for the immediate feedback. You are absolutely right, the backticks are a very elegant way to prevent evaluation. Thanks.