Skip to content
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

Simplification of dependencies #8

Open
mariodosreis opened this issue Nov 18, 2021 · 6 comments
Open

Simplification of dependencies #8

mariodosreis opened this issue Nov 18, 2021 · 6 comments

Comments

@mariodosreis
Copy link
Member

mariodosreis commented Nov 18, 2021

@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
Copy link
Member Author

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

@gaballench
Copy link
Contributor

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
Copy link
Member

sabifo4 commented Mar 14, 2024

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
Copy link
Member

sabifo4 commented Mar 14, 2024

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
Copy link
Member Author

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
Copy link
Member

sabifo4 commented Mar 27, 2024

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants