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

Homer package #216

Open
wants to merge 11 commits into
base: releases/paien
Choose a base branch
from
Open

Homer package #216

wants to merge 11 commits into from

Conversation

nvarini
Copy link

@nvarini nvarini commented Nov 8, 2018

No description provided.

@alalazo alalazo closed this Nov 9, 2018
@alalazo alalazo reopened this Nov 9, 2018
@alalazo
Copy link

alalazo commented Nov 9, 2018

Closed and reopened to trigger a new build in Travis.

Copy link

@rmsds rmsds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this package installation was a bit unorthodox I had a look at how it installs.

I see it also depends on perl (as most of the installed scripts are perl scripts).

And most of those have a section right at the beginning which points to a non existing path:

use lib "/gpfs/data01/cbenner/software/homer/.//bin";
my $homeDir = "/gpfs/data01/cbenner/software/homer/./";

This needs to be fixed, otherwise they all fail immediately.

Another question is the "extra" resources it gets based on the content of this file.
It would be nice to know if if the tool is at all useful if those are not installed, or it's possible to install them later in the user home directory. To understand if we should just install all of them.

Maybe this could be a variant, as the configureHomer.pl script seems to support either a list of packages -install <package name1> [package name2] or -all.

@rmsds
Copy link

rmsds commented Dec 1, 2018

I got some feedback from E.P.L.:

I don’t think homer accepts packages installed by users. He looks for the packages inside his folder structure. I don’t see an option to change that.
If you want to reduce the size, you can install only the organisms people are going to use. That would be human (version hg19) and mouse (version mm9 and mm10). You can ask if someone is interested in other organisms but I very much doubt it.

So, I think the way to go would be a variant with the list of organisms to install. In our case it should be "hg19,mm9,mm10". I'll still ask the other users which requested it to see if they need anything else just in case.

Copy link

@rmsds rmsds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides my suggested change for the variant (see below) I think you also accidentally picked up some changes in R packages.

depends_on('r-edger', type='run')
depends_on('r-deseq2', type='run')

variant('data', default=False,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the 'data' and 'custom' variants are effectively exclusive, I would prefer a multi-valued variant, maybe:

Suggested change
variant('data', default=False,
variant('data', default='none', description='Which genome data packages to download',
values=('none','all','minimal'))

With the matching logic in the install() below.

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

Successfully merging this pull request may close these issues.

None yet

6 participants