-
Notifications
You must be signed in to change notification settings - Fork 53
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
Implement mofa2 wrapper for MAE #144
base: master
Are you sure you want to change the base?
Conversation
The R script of interest is create_mofa_from_mae.R. I left |
Could you more clearly show here with an example how something can be done more efficiently with this new contribution? E.g. show parallel examples of current & new way. This will make it easier to see how this will be useful. |
Sure! The following example is inspired by Chapter 15 of the OMA book. Import and preprocess dataset:
Example with wrapper:
Same example without wrapper:
Then, the
|
Hmm seems more straightfwd and easy to implement. |
Up. |
@rargelaguet any thoughts on when this PR could be considered..? |
Hi, I'm also interested in this feature. Are you planning to merge? |
Hi @rargelaguet - any thoughts on when this PR could be considered..? |
Hi @antagomir and @RiboRings, thanks a lot for the pull request and I am very sorry for the delay on my side. I like the idea of having a general wrapper called [mofa2] that handles all input format and simplifies the process of creating the MOFA object. But the mofa2 function in the pull request only handles the MultiAssayExperiment format, which I think can be a bit misleading. I can extend the Thanks again for you feedback and contribution! |
Thank you for the reply! It sounds good to me. Feel free to let me know if anything should be changed/improved in the method for MultiAssayExperiment. |
Sounds good. It just came to my mind that SingleCellExperiment/TreeSummarizedExperiment objects have alternative experiments (altExp) that are also commonly used to store multiple data tables. If it is welcome, we could add support for that format. It is probably as important in practice as the MultiAssayExperiment. |
Hi @antagomir and @RiboRings, thanks again for your suggestions, very much appreciated. I have been exploring the option to implement a one-line wrapper function that creates a MOFA2 object from multiple input formats and that it also parses the data/model/training options. This is now implemented a function called Note in this function I have incorporated an argument called |
Hi @rargelaguet! Thank you for getting back to this discussion. For me, the
Maybe one option would be to just keep only |
Great. I made a quick test following the example on the manpage.
This throws the error: Error in names(assays) <- names(experiments(mae)) : When I drop the third object from MAE it works:
Checking data options... It might be less error-prone if the user was required to specify assay for all availble experiments, for instance like
or
using NULL to indicate experiments that should be dropped out from the analysis? |
If support for altExps could be added that would be great. The logic could be the same than for MAE (i.e. specifying the vector of assays, one for each altExp). Often altExp is the recommended choice when samples match one-to-one. I noticed that mofa2 function does not yet have @export tag |
Hi @rargelaguet! Any news when this could be merged? |
I think we could also help to finalize this by making a PR against the development branch, if useful. |
Hi @rargelaguet! Any update on this? |
I second @RiboRings, any updates @rargelaguet? |
.select_assays <- function(mae, assay.types) { | ||
# Give corresponding experiment names to assay.types | ||
names(assay.types) <- names(experiments(mae)) | ||
# For every experiment in MAE | ||
for ( exp in names(experiments(mae)) ){ | ||
# Keep only selected assay.type from a given experiment | ||
assays(mae[[exp]]) <- list(assay(mae[[exp]], assay.types[[exp]])) | ||
# Update assay names | ||
assayNames(mae[[exp]]) <- assay.types[[exp]] | ||
} | ||
return(mae) | ||
} |
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.
These can lead to an errors that might be hard for user to solve:
- MAE can have also other datatypes than SEs. The object just has to have rows and columns.
- What if the length of assay.types does not match with length of experiments?
Something like this might be more explicit with the assay names and experiments
|
# For every option in a set (data, model, train, ...) | ||
for ( opt in names(default) ){ | ||
# If that option is found among arguments | ||
if ( opt %in% names(list(...)) ){ | ||
# Replace default with value specified in arguments | ||
default[[opt]] <- list(...)[[opt]] | ||
} | ||
} |
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.
Came to my mind while resting, does this work to avoid loops?
user_options <- list(...)
set_options <- intersect(names(default), names(user_options))
default[set_options] <- user_options[set_options]
Any updates @rargelaguet? Also pinging @RiboRings. |
The current method for MultiAssayExperiments requires some manual work which the end user may not be familiar with. As in this example, after transforming assays you also need to:
assay(mae[[1]], "counts") <- NULL
)create_mofa_from_MultiAssayExperiment
model_opts$num_factors <- 5
)prepare_mofa
As an alternative to performing those 4 steps separately, I propose to include them in a wrapper function, whose result can be directly passed to
run_mofa
.