broadinstitute / viral-ngs

Viral genomics analysis pipelines
Other
187 stars 67 forks source link

problem installing Krona in 1.4.2 #321

Closed sbecuwe closed 8 years ago

sbecuwe commented 8 years ago

Hello,

After adding the requirements-test of #315, I was able to run py.test -v test/unit/test_tools.py However, a problem shows up: unable to install Krona. [gw0] FAILED test/unit/test_tools.py::test_tool_install[Krona] I found URLs in many files in the tools directory, but nothing is mentioned in krona.py. Am I overlooking something? How should I get Krona installed? Thanks again!

yesimon commented 8 years ago

Are there any more detailed logs? What about your OS and system information?

sbecuwe commented 8 years ago

I'm installing it on Scientific Linux release 6.7. This is the log:

========================== FAILURES ==================== _____ test_tool_install[Krona] __ [gw0] linux -- Python 3.5.1 [...]/Python/3.5./bin/python tool_class = <class 'tools.krona.Krona'>

def test_tool_install(tool_class):
    t = tool_class()
    t.install()
   assert t.is_installed(), "installation of tool %s failed" % tool_class.__name__

E AssertionError: installation of tool Krona failed E assert <bound method Tool.is_installed of <tools.krona.Krona object at 0x2b19b19103c8>>() E + where <bound method Tool.is_installed of <tools.krona.Krona object at 0x2b19b19103c8>> = <tools.krona.Krona object at 0x2b19b19103c8>.is_installed

test/unit/test_tools.py:48: AssertionError ----------------------------------- Captured stdout setup ------------------------------------------- <class 'tools.krona.Krona'> ============= 1 failed, 39 passed in 3.89 seconds ==========================

yesimon commented 8 years ago

We definitely need to tweak the logging to get better diagnostics to see why it's failing on your machine.

sbecuwe commented 8 years ago

I assume it's Krona from this website. Is that correct? I install viral-ngs as a regular user in a dedicated directory, say $VIRALNGS, not in default locations as /usr/local, /opt etc. I guess following steps have to be taken: wget https://github.com/marbl/Krona/releases/download/v2.6.1/KronaTools-2.6.1.tar tar xf KronaTools-2.6.1.tar cd KronaTools-2.6.1 ./install.pl --prefix=$VIRALNGS/tools/build/krona -taxonomy $VIRALNGS/tools/build/krona/taxonomy or wherever the taxonomy directory is supposed to be PATH=$VIRALNGS/tools/build/krona/bin:$PATH ./updateTaxonomy.sh $VIRALNGS/tools/build/krona/taxonomy/ However, taking a look at tools/krona.py, I find patches=[('opt/krona/updateTaxonomy.sh', opt = os.path.abspath(join(bin_path, '..', 'opt', 'krona')) so I guess you assume krona is already installed in a specific */opt/* location. I don't find any URL in krona.py, so no download is initiated from within this script. I guess a class DownloadAndBuildKrona(tools.DownloadPackage) should be added to make it more generic?

dpark01 commented 8 years ago

Hi @sbecuwe, can you show us the output of typing: conda list -p {path to your viral-ngs git clone copy}/tools/build/conda-tools/default

sbecuwe commented 8 years ago

I don't have conda installed. Installing 1.3.5/1.3.6 went fine without it. As indicated on the webpage "Most tools will attemp a conda-based install first, before falling back to an install handled entirely by our wrappers." so fall back worked in the past. I guess the fall back procedure could work if a DownloadAndBuildKrona(tools.DownloadPackage) would be there, is that correct? Could you also confirm that the KronaTools I mentioned is the correct one?

yesimon commented 8 years ago

Krona only has a conda install method, so what you said is absolutely correct. Unless you're using metagenomics tools you wouldn't need Krona, so for now we'll probably make the install optional.

sbecuwe commented 8 years ago

Ok, thanks for the info! It seems the metagenomics tools are not needed (yet).

dpark01 commented 8 years ago

Okay, FYI, we're generally moving towards the direction of requiring conda for the tool dependencies and may eventually deprecate the manual installation machinery. We haven't yet, but for example, we haven't bothered to implement DownloadAndBuild type code for the newer tools in our toolset.

sbecuwe commented 8 years ago

Thanks for the info. I just tried travis/install-conda.sh to install miniconda and it worked without a problem, including py.test -v test/unit/test_tools.py. One suggestion: numpy seems to be a requirement for biopython, so I would suggest adding numpy to requirements.txt.

dpark01 commented 8 years ago

Hm, that's a surprise (maybe it's new). But if numpy is a true prerequisite for biopython, biopython should list it as a requirement in their setup.py and pip/pypi will handle it automatically as it always does. I don't think it's a requirement for anything we use biopython for, however.

sbecuwe commented 8 years ago

See attached file for more info. pip.txt

dpark01 commented 8 years ago

From the pypi page:

This package is only used in the computationally-oriented modules. It is required for Bio.Cluster, Bio.PDB and a few other modules. If you think you might need these modules, then please install NumPy first BEFORE installing Biopython.

There's actually a ton of other optional dependencies in biopython, but we don't use any of those features (nor do we use Bio.Cluster or Bio.PDB). We use very basic file I/O and sequence libraries in biopython, and these are all unit tested on our end. I think we'll skip numpy, it adds 10-15 mins to the build process.