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

Vep #22

Merged
merged 17 commits into from
Oct 7, 2020
Merged

Vep #22

merged 17 commits into from
Oct 7, 2020

Conversation

illusional
Copy link
Member

Adding Vep to Janis-bioinf. This is a bit complicated and may require some rework for plugins.

@matthdsm
Copy link
Contributor

Quick question,

Why not make your life easier and use the bioconda version of VEP + container?
All plugins are already included there and the docker image is pre made.

Cheers
M

@illusional
Copy link
Member Author

Thanks @matthdsm, that's exactly what I was looking for!

I think this will need some extra features to janis-core to ensure correct binding of plugin dependencies while creating the command line, but the bioconda docker is exactly what I wanted, and you built it!

@matthdsm
Copy link
Contributor

conda has images for virtually every piece of software registered there, so it's an easy to use resource (cfr my PR, also conda dockers).

Cheers
M

@illusional
Copy link
Member Author

Yeah agreed, my first port of call is usually the original vendor for a container, and then biocontainers. But the biocontainers is definitely more what we want here.

# Conflicts:
#	janis_bioinformatics/tools/ensembl/vep/v98_3/base.py
@illusional
Copy link
Member Author

So the plugins have always been a bit of a complication, but now with expressions (https://github.com/PMCC-BioinformaticsCore/janis-core/pull/10/files), we're one step closer to this being merged.

The syntax for a plugin looks like the following:

ToolArgument(
    If(
        IsDefined(InputSelector("caddReference")),
        [
            "--plugin",
            "CADD," + JoinOperator(InputSelector("caddReference"), ","),
        ],
        None,
    )
),

I'm waiting on clarifying the WDL separator function (and obviously the PR expression to merge) and then I can test this a bit more.

The reality is every plugin effectively needs to be added to the wrapper separately, not a big deal but something to keep in mind. I've added one (1) thing for a custom plugin, that block could be duplicated but is a little hard to make generic (feedback welcome)!

@illusional
Copy link
Member Author

This requires PMCC-BioinformaticsCore/janis-core#10 to be completed before this can be merged.

@illusional
Copy link
Member Author

@rlupat this is probably ready to merge with janis-core changes already in master and released.

@illusional illusional merged commit a152e6c into master Oct 7, 2020
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.

None yet

2 participants