morinlab / GAMBLR

Set of standardized functions to operate with genomic data
https://morinlab.github.io/GAMBLR/
MIT License
3 stars 2 forks source link

Resolving issues, bugs, and making reproducible installation #146

Closed Kdreval closed 1 year ago

Kdreval commented 1 year ago

This PR fixes issues #155 and #154. The autoinstallation has been tested both on Mac and Linux-based systems and the set of command that worked is:

conda create -n gamblr-dependencies r-base=4.1.3 r-devtools r-biocmanager cmake r-rmysql r-git2r mysqlclient r-tidyverse r-nloptr r-xml -c conda-forge -c anaconda

conda activate gamblr-dependencies

R --vanilla

devtools::install_github("morinlab/GAMBLR@kdreval-dev")

In addition to this, there are updates to the GAMBLR color palette because one of the palettes was having duplicated colors for 2 different genetic subgroups. The prettyForestplot has also now functionality to report the number of mutated and unmutated patients in the output of the Fisher test results. There are also some other minor bug fixes in FtestCNV and adjust_ploidy functions.

All documentation was updated accordingly and the most recent master was merged to ensure no conflicts.

Kdreval commented 1 year ago

This is ready for review!

Kdreval commented 1 year ago

This PR has the master merged and is using the latest update to support most complete SV results when running Chapuy classifier. All other comments were also addressed and marked as resolved. It is ready for merge

Kdreval commented 1 year ago

Also confirmed the PR approved yesterday has no merge conflicts with this one

Kdreval commented 1 year ago

This PR now also fixes the bug of randomForest dependency auto-installation reported in the issue #154

Kdreval commented 1 year ago

In this PR now I have moved out the functionality of classifications and predictions to it's own designated R package GAMLR.predict. It is ready for review and merge if all looks good. Thanks!

mattssca commented 1 year ago

In addition, I also think prettyForestPlot is using stats functions (p.adjust) and prettyOncoplot might also be using the same package (calling the setNames function). Thoughts?

Kdreval commented 1 year ago

When doing this update I found the stats package is one of the r default packages. It is always loaded on startup and is always available without explicitly loading it. When it was added as a dependency, it only created issues with name conflicts etc. Now having this removed makes the GAMBLR easier to load and instead of ~40 warnings on startup, there are only ~ 20. I would think we do not need to revert it back to being explicitly mentioned in NAMESPACE and DESCRIPTION.

mattssca commented 1 year ago

Sounds good Kostia, thanks for looking into it!

Kdreval commented 1 year ago

@rdmorin the merge is blocked even after approval because earlier some changes were requested. Can you please look into this?

rdmorin commented 1 year ago

Should work now

Kdreval commented 1 year ago

thanks!!