dosreislab / mcmc3r

mcmc3r: an R package for MCMCTree
MIT License
2 stars 2 forks source link

Simplification of dependencies #8

Open mariodosreis opened 2 years ago

mariodosreis commented 2 years ago

@sabifo4 Dependencies for the package should be simplified to make it easier to install and maintain the package. Dependency Morpho should be moved from Imports into Suggests. Functions using the Morpho package should check if the package is installed and produce an error message if not. Please see https://r-pkgs.org/description.html for clear explanation of how to do this.

mariodosreis commented 2 years ago

@sabifo4 any progress on this? Could you list here which functions depend on the Morpho package, to decide how to deal with them?

gaballench commented 1 year ago

Just mentioning here that due to the dependency Morpho which in turn depends on sf, it ends up requiring gdal-dev at the system level, which is e.g. difficult to install using conda. As conda environments are popular among users of HPC systems, it makes hard to install mcmc3r.

If you agree I can help to take a look at what exactly requires Morpho and see if it can be moved into Suggests without breaking something.

sabifo4 commented 7 months ago

Package Morpho relies on various dependencies as per the CRAN docu, which I understand is quite hard to maintain.

There are two Morpho functions that are required for mcmc3r::proc2MCMCtree to work (main function used to generate the morpho alignment): Morpho::procSym and Morpho::align2procSym. If Morpho is moved to Suggests, this function will crash I am afraid.

sabifo4 commented 7 months ago

Update following @mariodosreis suggestion (apologies, missed that!) about launching an error when Morpho is missing:

 # First lines in function `proc2MCMCtree` in `morpho.R` script
 # 240314-SAC
 # Check for Morpho package and, if not available, stop and ask the user
 # to install it -- Morpho has been moved to `Suggests` in DESCRIPTION file
 is_Morpho_installed <- c( "Morpho" %in% rownames(installed.packages()) )
 if( is_Morpho_installed != TRUE ){
   stop( "\nPlease install R pacakge \"Morpho\" to run \"proc2MCMCtree\":\n install.packages(\"Morpho\")" )
 }

I am checking other parts of the code before submitting a pull request, but writing this here just in case you need to quickly fix it!

mariodosreis commented 6 months ago

Hi @sabifo4, looking at imports in the package, I noticed Rdpack is also required, but doing a quick check, I couldn't find any functions using this package. Can we remove this dependency as well? When submitting a pull request, please submit only a fix to the dependencies issue. For other problems, we open a new issue and we submit pull requests separately. That makes review and testing of pull requests focused and easy to carry out. (see https://youtu.be/B_HR2R3xsnQ, don't mind the silly title, it is a nice video).

sabifo4 commented 6 months ago

I have made some changes in the code to address this issue! You can check now this pull request to double check these changes!

Once everything is checked and dev merged with master, this issue can be closed :)