Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding referenceGroup and treatmentGroup arguments, and updating plot #31

Closed
wants to merge 1 commit into from

Conversation

asyakhl
Copy link
Collaborator

@asyakhl asyakhl commented Sep 9, 2023

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

@asyakhl asyakhl requested a review from LiNk-NY September 9, 2023 23:43
@LiNk-NY
Copy link
Contributor

LiNk-NY commented Sep 11, 2023

@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
Copy link
Collaborator Author

asyakhl commented Sep 17, 2023

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

@LiNk-NY
Copy link
Contributor

LiNk-NY commented Sep 25, 2023

@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
Copy link
Member

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
Copy link
Contributor

LiNk-NY commented Oct 19, 2023

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

@LiNk-NY LiNk-NY closed this Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants