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

iGenomes fix #123

Merged
merged 3 commits into from
Nov 26, 2019
Merged

iGenomes fix #123

merged 3 commits into from
Nov 26, 2019

Conversation

ewels
Copy link
Member

@ewels ewels commented Nov 25, 2019

Fixes #121 by moving the reference genome params back in to main.nf. This means that they are set after the iGenomes config is loaded.

@ewels ewels requested review from phue and a team November 25, 2019 15:53
@phue
Copy link
Member

phue commented Nov 25, 2019

Apologies for breaking this, should have tested with igenomes 🤦‍♂️

@ewels any chance you could squeeze in the memory adjustments for TrimGalore here aswell?

@ewels
Copy link
Member Author

ewels commented Nov 25, 2019

No problem! I didn’t spot it either.. Memory changes went in same time as your comment :)

@alneberg
Copy link
Member

Travis reports some strange MultiQC error:

  Traceback (most recent call last):
    File "/opt/conda/envs/nf-core-methylseq-1.5dev/bin/multiqc", line 6, in <module>
      from multiqc.__main__ import multiqc
    File "/opt/conda/envs/nf-core-methylseq-1.5dev/lib/python2.7/site-packages/multiqc/__main__.py", line 44, in <module>
      multiqc.run_cli(prog_name='multiqc')
    File "/opt/conda/envs/nf-core-methylseq-1.5dev/lib/python2.7/site-packages/click/core.py", line 764, in __call__
      return self.main(*args, **kwargs)
    File "/opt/conda/envs/nf-core-methylseq-1.5dev/lib/python2.7/site-packages/click/core.py", line 717, in main
      rv = self.invoke(ctx)
    File "/opt/conda/envs/nf-core-methylseq-1.5dev/lib/python2.7/site-packages/click/core.py", line 956, in invoke
      return ctx.invoke(self.callback, **ctx.params)
    File "/opt/conda/envs/nf-core-methylseq-1.5dev/lib/python2.7/site-packages/click/core.py", line 555, in invoke
      return callback(*args, **kwargs)
    File "/opt/conda/envs/nf-core-methylseq-1.5dev/lib/python2.7/site-packages/multiqc/multiqc.py", line 238, in run_cli
      kwargs=kwargs
    File "/opt/conda/envs/nf-core-methylseq-1.5dev/lib/python2.7/site-packages/multiqc/multiqc.py", line 517, in run
      template_mod = config.avail_templates[config.template].load()
    File "/opt/conda/envs/nf-core-methylseq-1.5dev/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2442, in load
      self.require(*args, **kwargs)
    File "/opt/conda/envs/nf-core-methylseq-1.5dev/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2465, in require
      items = working_set.resolve(reqs, env, installer, extras=self.extras)
    File "/opt/conda/envs/nf-core-methylseq-1.5dev/lib/python2.7/site-packages/pkg_resources/__init__.py", line 786, in resolve
      raise DistributionNotFound(req, requirers)
  pkg_resources.DistributionNotFound: The 'monotonic; python_version == "2.6" or python_version == "2.7" or python_version == "3.0" or python_version == "3.1" or python_version == "3.2"' distribution was not found and is required by humanfriendly

Any clues?

@phue
Copy link
Member

phue commented Nov 26, 2019

strange indeed, especially because tests have passed before with MultiQC 1.8 ( see #120 )

What I also find strange is that conda seems to create a python 2 environment by default, is this expected behaviour?

@ewels
Copy link
Member Author

ewels commented Nov 26, 2019

What I also find strange is that conda seems to create a python 2 environment by default, is this expected behaviour?

Hmm, I guess that this is because the nfcore/tools image uses continuumio/miniconda:4.6.14 which is Miniconda 2 - see https://hub.docker.com/r/continuumio/miniconda/

Docker container with a bootstrapped installation of Miniconda (based on Python 2.7) that is ready to use.

We could use Miniconda3 instead if we want, which is based on Python 3.5 - see https://hub.docker.com/r/continuumio/miniconda3

BUT - I don't think that this should matter? The base python version of conda should be separate from the python version in the environment. So it's perfectly possible to run Python 3x in a miniconda2 environment.

@ewels
Copy link
Member Author

ewels commented Nov 26, 2019

strange indeed, especially because tests have passed before with MultiQC 1.8 ( see #120 )

The reason that this passed is because the tests pull the docker image from dockerhub. This image isn't updated until after the PR is merged, so if you update software then the tests for that PR run on the old software still.

The only way to avoid this is to build the docker image in the tests, and that takes ages.. 😞

@ewels
Copy link
Member Author

ewels commented Nov 26, 2019

Note that because of the lag between PRs and docker builds, even if my commit above fixes the installation error, the tests on this PR will still fail, because it's using the faulty docker container. So we need to take a gamble and merge if we trust it, then wait a bit, then open a new PR to be sure that it's worked.

@alneberg
Copy link
Member

Right, will you be able to test it on the dev branch at least? A broken dev branch for a few moments is not the end of the world I guess.

@ewels
Copy link
Member Author

ewels commented Nov 26, 2019

Yup 👍

And as to why MultiQC is failing, I'm still not sure. It should work fine for Py2.7 still for this release at least. But we should be using Py3k anyway and I'm pretty confident that this will fix the error..

@ewels
Copy link
Member Author

ewels commented Nov 26, 2019

Ok, merging. Will open a new PR shortly with changes for #124 as it's a minor issue and that can act as the test for this fix..

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

Successfully merging this pull request may close these issues.

3 participants