search

NelleV / moanin

Timecourse transcriptomic analysis
https://nellev.github.io/moanin/
Other
5 stars 0 forks source link

Problem in running DE_timepoints if factor levels of group have a `:` or other illegitimate character #81

Open epurdom opened 4 years ago

epurdom commented 4 years ago

If the Group variable takes on values that have a character value that is not allowed by make.names (i.e. the function that checks for valid column header names for a data.frame), then the WeeklyGroup variable that is internally created won't match the Group variable names, because the definition of WeeklyGroup applies make.names to the group variable before combining with timepoints variable (line 187 in AllClasses.R)

    if(!("WeeklyGroup" %in% colnames(colData(data)))){
        colData(data)$WeeklyGroup = as.factor(
            make.names(colData(data)[,group_variable_name]:as.factor(
                colData(data)[,time_variable_name])))
    }

I'm not really sure why we do this, but this creates a problem is Group is created by pasting together factors, e.g.

Group<-Factor1:Factor2

At a minimum, we should apply make.names to the Group variable rather than only when we define WeeklyGroup. But if we don't need make.names then it would be better to get rid of it so that we aren't changing the data of the user.

epurdom commented 4 years ago

@NelleV I'm okay with fixing this in the package, but can you weigh in on why you used make.names to begin with here and whether/why we need to keep that?

NelleV commented 3 years ago

Hi Elizabeth, I don't remember why I did this. I'm not sure we need to keep this: I think I just reused some code from an example somewhere, and never gave more thought to this line of code.

epurdom commented 3 years ago

I'm wondering if these names become the header of a data frame output at somepoint, even just as intermediate step? I think removing make.names will actually need to be delicately tested, but I'll look into it.

cmatKhan commented 2 years ago

I just ran into this issue with Timepoints which have a starting point of -1. Switching them to 0 fixed the issue, but either a specific note in the docs/vignette (which I very well may have missed) or some way of specifically alerting the user would be nice.

background :

In this experiment, timePoint 0 was unmeasured. It was the point at which a sugar source was flowed into a medium. timePoint -1 was a measurement from before timePoint 0, which I realize is not really a timePoint at all. I think the best solution to this particular issue is just a note in the docs, or some error handling in the code, to tell the user "timepoints need to be 0 or greater decimals, silly".