search

waldronlab / lefser

R implementation of the LEfSe method
https://waldronlab.io/lefser/
38 stars 6 forks source link

Adding referenceGroup and treatmentGroup arguments, and updating plot #31

Closed asyakhl closed 8 months ago

asyakhl commented 10 months ago

Added referenceGroup and treatmentGroup arguments, code works well after testing.

tested
LiNk-NY commented 9 months ago

@lwaldron My preference is to have groupCol = "GROUP" be a factor in the data, output a message to the user what the reference value is, and clean up the code that uses 0/1 coding rather than have additional arguments (referenceGroup and treatmentGroup) added to the function. Note that these arguments have to be specified every time. What do you think?

asyakhl commented 9 months ago

@LiNk-NY That works, should referenceGroup and treatmentGroup be kept as optional?

LiNk-NY commented 9 months ago

@LiNk-NY That works, should referenceGroup and treatmentGroup be kept as optional?

IMO, neither should be used and the reference should be taken from the variable's levels, e.g.,

levels(as.factor(letters[1:5]))[1]
## OR better
contrasts(as.factor(letters[1:5]))
lwaldron commented 9 months ago

My preference is to have groupCol = "GROUP" be a factor in the data, output a message to the user what the reference value is

I agree with @LiNk-NY since R already has this feature built-in to the factor class. The user then only has to specify which is the "group" column using groupCol = "GROUP". I also agree with outputting a message to the user what the reference value is; the message could be like this:

msg <- c("The outcome variable is specified as '", 
             groupCol, 
             "' and the reference category is '", 
             levels(as.factor(relab[[groupCol]]))[1], 
             "'. See `?factor` or `?relevel` to change the reference category.")
message(msg)
LiNk-NY commented 8 months ago

Hi Asya, @asyakhl I simplified the code a bit adn incorporated the feedback above into the plot_groups branch. If there is anything I missed, please create a new PR based on the current plot_groups branch. Best, Marcel