cumc / pecotmr

Pair-wise enrichment, colocalization, TWAS and Mendelian Randomization to integrate molecular QTL and GWAS.
Other
12 stars 15 forks source link

Installation instructions are very much incomplete #270

Open pcarbo opened 1 day ago

pcarbo commented 1 day ago

When I try to install pecotmr, I get this error:

ERROR: dependencies ‘mr.ash.alpha’, ‘mr.mash.alpha’, ‘udr’, ‘snpStats’, ‘fsusieR’, ‘mvsusieR’, ‘qvalue’ 
are not available for package ‘pecotmr’

These packages are not available on CRAN which is why the installation fails. I would recommend using the Remotes: in DESCRIPTION to tell remotes::install_github where to find these packages. See here for an illustration, and see here for details.

Once you have updated your DESCRIPTION I can check that it works.

Unrelated, I got a second installation error that is due to some installation difficulties with Rfast. I eventually got it working, but I'll note that the system requirements are quite stringent — Rfast will be quite difficult to install for many people. If you can make it an optional depenency, that would help a lot.

kevinlkx commented 1 day ago

I got an installation error using micromamba install r-pecotmr -c dnachun

error    libmamba Could not solve for environment specs
    The following package could not be installed
    └─ r-pecotmr is not installable because it requires
       └─ bioconductor-iranges >=2.36.0,<2.37.0a0, which does not exist (perhaps a missing channel).
critical libmamba Could not solve for environment specs
danielnachun commented 1 day ago

Thanks for noting these issues.

qvalue and snpstats are Bioconductor packages so I need to see how to make sure that these get pulled from there correctly.

We are no longer using udr, so that should be removed.

mr.ash.alpha, mr.mash.alpha, fsusieR and mvsusieR should all be handled with remotes as you described. We hoping these packages can all be submitted to CRAN in the near-ish future (we're happy to try to help with that!).

@kevinlkx I believe this is because it should be micromamba install r-pecotmr -c dnachun -c conda-forge -c bioconda and I see our instructions for this are incorrect, so I will fix that too.

danielnachun commented 1 day ago

@pcarbo Can you also share the failure you experienced with Rfast? I think this can be made optional as we are using for faster LD calculation (though it makes a big difference). I'm guessing it needs a newer C++ compiler, and in the era of conda-forge this is actually quite easy to provide instructions for.

danielnachun commented 1 day ago

These issues should be fixed by https://github.com/cumc/pecotmr/pull/271 and https://github.com/cumc/pecotmr/pull/272

pcarbo commented 1 day ago

Can you also share the failure you experienced with Rfast?

This is the issue with Rfast:

SystemRequirements: C++17

CRAN strongly discourages such requirements because it many platforms by default do not support this requirement (that is, with standard system compilers).

So my suggestion is to make it a "suggested" package, and add a strongly worded message if it is not being used to encourage people to install it (noting that computations can be very slow if it is not used). This is the approach we have taken in other packages such as susieR.

pcarbo commented 1 day ago

@danielnachun Your new installation instructions aren't working for me:

> BiocManager::install("cumc/pecotmr",update=FALSE,ask=FALSE)
ERROR: dependencies ‘mr.ash.alpha’, ‘mr.mash.alpha’, ‘fsusieR’, ‘mvsusieR’ are not available for package ‘pecotmr’
danielnachun commented 1 day ago

I wasn't able to test this before in a totally clean system but now that I have I found a few different issues with the full source build. I'm working on the fixes now - not only did I specify the Remotes: section incorrectly, but we also have some suggested packages that are currently not actually optional.

I'll work on making more packages optional or removing some dependencies all together:

danielnachun commented 1 day ago

@pcarbo I've now gotten everything to fully build from source using BiocManager, so please give it a try again. Once we've pruned the dependency tree better and the development cycle of this package isn't as fast I will enable testing of full source builds for Linux in our CI pipeline. For macOS I'm going to assume anyone installing it is using the official R.app distribution that uses binary packages. Windows support testing is the lowest priority but will still eventually be added to the CI pipeline, under the same assumption as macOS that the user is getting binary packages when possible.