-
Notifications
You must be signed in to change notification settings - Fork 27
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
Initialise mediation functions #503
Conversation
Great.
|
Looks nice!
In this case one options would be
|
Thank you for the comments! @TuomasBorman, if I understand correctly, you are suggesting that mediateColData and mediateAssay should be combined into a single function, right? It sounds like it would simplify the code, and make it possible to use single or multiple variables for any of the outcome, mediator and treatment arguments. But then how should we standardise the output? Because when multiple analyses are run (mediateAssay), the most practical output is a data.frame of model statistics for every combination, whereas for a single analysis (mediateColData), the most informative output is the trained model itself (so you can do |
@antagomir, so you would consider this implementation with mediation::mediate like one option out of many, right? And then other mediation functions such as the hdmed ones could be specified with In this case, how would you control the suitability of the custom method? For example, the hdmed (high-dimensional mediation) funs are only appropriate for high-dimensional data like the assay (mediateAssay), but not for colData variables (mediateColData). Also for this aspect having two separate functions might be more convenient(?) Or actually what do you mean by custom mediation functions? |
Well in fact also the arguments may be very different for different methods. That's why there is for instance runMDS, runNMDS, etc. and not a single runOrdination.. so perhaps it is not good to go very generic for now. Let's just finalize this for the mediation package. Regarding comment from TB above: it is recommened in general that the function output should always be the same, regardless of the inputs. We could have that, and in addition the user could choose to include the resulting model/s in metadata slot (default FALSE but examples could show how to do this when necessary). This applies also to the assay case, it would be a list of output models. Would that work? Default output could indeed be a data frame with effect sizes and (adjusted) p-values. |
Hi! The Caveats:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still have to run the code, but these are what I found by just looking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the review comments. Overall, looks nice!
Is there any useful way to visualize mediation results? For the examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked the code now in detail, seems good and working
@RiboRings if you can close the conversations one by one when they have been resolved that would be great, to keep track on what was solved and what not |
Up - @RiboRings |
Hi! I finally got back to this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Top quality!
I forgot to mention that update NEWS file, It should include all the changes in easy, human-readable format. --> Added getMediation and addMediation functions.
All options ok to me. hitchip can move permanently to mia if this helps, there is no particular need to have it in miaTime. It was put there because we needed to limit the number of data sets in mia package and because hitchip had some time series in it |
Maybe we could comment out the examples and remove miaTime from DESCRIPTION, and then uncomment the examples when miaTime is submitted to BioConductor? |
That was actually same to where I ended up after I thought this over night. --> So let's do that. Instead of commenting out do:
miaTime is used some other places also, but they are not affected. They are already ran only if miaTime is installed. (vignette is not ran) |
a shining idea |
R CMD check fails due to warning:
Also, for some reason miaTime is being installed even when it is not in DESCRIPTION:
|
For me running the latest version of the "mediation" branch through checks throws warning
Also
[9] 2.5067 - 2.5100 == -0.00331
|
I added \donttest which should fix the issue of @RiboRings. @antagomir I cannot reproduce your error. It runs fine in my local machine, and the same tests are also run in GHA. Although, it is good to investigate this further to get answer on why you get different results |
Did not run, I disabled the examples |
If this goes through automated tests then I think should be ok |
Ubuntu:
Mac:
|
That will be hopefully fixed soon when new Bioc is released. However R 4.5.0 is little bit odd since next Bioc devel is developed against 4.4.0. That is something to do with rworkflows, I think |
Hi @TuomasBorman! Should I do anything to fix this? How should we proceed to merge this PR? |
I haven't been able to fix the problem, however, everything should be ok if at least one of these runs work and there are no errors related to this PR. --> so we can merge without passing all these rules (I asked from rworkflows maintainers if they know what is the problem, and they have not answered yet) |
Seems ok, I will merge (I just updated NEWS). Thanks! |
This PR is not ready yet, but it was opened to begin discussion around mediation analysis for mia. Here, two new main functions are introduced: mediateColData and mediateAssay, both located in R/mediate.R. They depend on the mediation package and if this is not ideal these functions could be placed in an extension of mia.
mediateColData provides an easy way to run mediation analysis between 3 variables in the colData, while mediateAssay allows to run multiple mediation models for every taxon in an assay, or for every component of a reducedDim, and subsequently adjust significance for multiple comparison. They both leverage the mediation::mediate function under the hood.
mediateColData takes a tse object, three colData variables for outcome, treatment and mediator, respectively, plus other covariates from the colData and other arguments supported by mediation::mediate. It returns the output of the mediation model (similar to the output of lm or glm). Below is a self-contained example of how it works:
mediateAssay iterates over the same code used for mediateColData. It takes the extra arguments assay.type and dim.type. If one of them is correctly specified, it runs mediation::mediate for every row of an assay or every column of a reducedDim slot, respectively. As output, it returns a dataframe with effect sizes and p-values for every taxon/component. Below is an example that requires variables from the previous example above:
More examples can be found in this repository