Conversation
mtiberti
left a comment
There was a problem hiding this comment.
can you also please
- detail the change in the vignette; we don't necessarily need to add a call to it using the new method, just modifying the already-existing call so that we use the old method explicitly, and add a bit of an explainer of why we support both
- can you add a test with either mode in the test suite (one for ora, one for fgsa)
| #' This function carries out the functional enrichment analysis (FEA) | ||
| #' @param BPname BPname biological process such as "proliferation of cells", "ALL" (default) if FEA should be carried out for all 101 biological processes | ||
| #' @param DEGsmatrix DEGsmatrix output from DEA such as dataDEGs | ||
| #' @param DEGsmatrix output from DEA such as dataDEGs or from DAA such as differential abundace matrix |
There was a problem hiding this comment.
can we also add the name of the object as in dataDEGs?
ideally both the name of the object and explanation of what they are for both
There was a problem hiding this comment.
About this, I have a doubt: as far as I understood- the dataDEGs argument is defined for the first time in the moonlight.R, specifically in the moonlight() wrapper function when the dataDEGs argument is passed in the method. So, If I am correct- to be consistent- we should define a more generic data table parameter name (that would refer to both genes and proteins tables since we now have the support for the DAPsmatrix) also in the moonlight.R. But since dataDEGs is used in many other functions of Moonlight- this would imply to change the input object name in all the other functions that takes as input the dataDEGs object, but I am not sure on how to proceed on this.
Another option would be to define two separate objects: one for proteomics and one for transcriptomics and use the proteomics object only in the FEA() function.
What do you think about it?
There was a problem hiding this comment.
I would avoid the second option - if we end up in a situation in which our function with two (mostly) separate code paths and two independent arguments that use the two code paths, then it should be split into two independent functions.
I don't think we should change how the moonlight() wrapper works before we have a full understanding of the new "proteomics" pipeline (or other functions) will work from start to end. But I agree that it would be better to define a more generic argument name for the data for this function.
Maybe we can start by defining a new more generic parameter name in this function and change moonlight.R accordingly (and potentially on other functions that run FEA if any), until we have a good idea of what else we need to change
There was a problem hiding this comment.
we should update this and following errors as we don't consider just DEGs anymore
| @@ -2,8 +2,10 @@ | |||
| #' | |||
| #' This function carries out the functional enrichment analysis (FEA) | |||
There was a problem hiding this comment.
can you add a bit more to the description - the fact that there are two possible methods and what inputs they are expected to be used
… results, added new test for fgsea
| For each DEG in the gene set a z-score is calculated. This score indicates how the genes act in the gene set. | ||
|
|
||
| To perform the analysis, the user can choose among two different methods: | ||
| the default method is "ora" and it is based on Fisher's test. It is used to tests whether a gene set is overrepresented in a subset of DEGs or DAPs. |
There was a problem hiding this comment.
have we defined what the acronym of DAP/DEG is?
| Zscore <- 0 | ||
| } | ||
|
|
||
| return (Zscore) |
There was a problem hiding this comment.
super nitpick - can you remove the space between return and (Zscore)?
| # Moonlight2R 1.9.1 | ||
| # Moonlight2R 1.9.2 | ||
|
|
||
| ## Summary | ||
|
|
||
| * Removed hotfix in GSEA() module | ||
| * Added new dataset for proteomics data and implemented fgsea method in the FEA function |
There was a problem hiding this comment.
here you have replaced the entry for 1.9.1 with the one for 1.9.2 - can we keep both?
A new dataset called
DAPsmatrixis added indata/and documented inRdata.R. The dataset contains Differentially Abundant Proteins along with logFC and p values.The method
fgseafor functional enrichment analysis has been added in theFEA()module. TheFEA()can now take as input bothDEGsmatrixandDAPsmatrixand a newmethodkeyword, indicating which Functional Enrichment Analysis method will be used. The default method is called"ora"and is associated with the already present implementation of the Over Representation Analysis method. A new method has been added called"fgsea"which perform FEA using a ranking-based method. The final output in both cases would be the TableDiseaseNew containing information about which BP was found to be enriched in the input DAPs or DEGs matrix. When running thefgseamethod, the output table will contain different values compared to theoraoutput, reflecting the specific scores (e.g. ES, NES) computed by the method itself. In both cases, aMoonlight Z-scorewill be computed for each BP and included in the output table. Its calculation in both method follows the same logic.FEA.Rd was updated accordingly
DESCRIPTION was updated with the version number and to include the new
fgseapackageNAMESPACE was updated to include the import of the
fgseapackage