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

Fix (=really add) Clinker #12472

Closed
wants to merge 26 commits into from
Closed

Conversation

andrewlonsdale
Copy link

  • I have read the guidelines for bioconda recipes.
  • This PR adds a new recipe.
  • AFAIK, this recipe is directly relevant to the biological sciences (otherwise, please submit to the more general purpose conda-forge channel).
  • This PR updates an existing recipe.
  • This PR does something else (explain below).

These are minor changes intended to trigger the build process again as previous addition of this recipe passed tests and was merged into master (#10244) but the recipe does not appear to be available.

Update to build as recipe not appearing in bioconda packages
@epruesse
Copy link
Member

epruesse commented Dec 7, 2018

The build log shows "nothing to be done". This still won't build anything.

@epruesse
Copy link
Member

epruesse commented Dec 7, 2018

Still "nothing to be done". Do not merge.

@andrewlonsdale
Copy link
Author

Thanks for looking at this @epruesse

I'm still unclear what is going on with this recipe. Can you advise on what is preventing this recipe from appearing in bioconda, i.e not available from conda or here https://bioconda.github.io/recipes.html, but appears in the master branch.

@tfursten
Copy link
Member

@andrewlonsdale I had this 'nothing to be done' issue, I think it was caused by naming the file meta.yml instead of meta.yaml.

@epruesse
Copy link
Member

Aaaah! Thanks @tfursten - that seems to be the case here too.

@andrewlonsdale
Copy link
Author

Thanks @tfursten and @epruesse! That 'meta.yml' fix was helpful, revealing underlying issues with recipe. I've had some timeout issues with local circleci as we as the auto github tests, latest update hopefully addresses these.

@epruesse
Copy link
Member

epruesse commented Jan 8, 2019

You probably have something in the recipe that gets stuck. Your job timed out after 5 hours of processing.

- samtools
- star ==2.5.3a
- bioconductor-biomart ==2.34.2
- bioconductor-gviz ==1.22.3
Copy link
Member

Choose a reason for hiding this comment

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

If you need to pin that tightly, you should distribute an environment file. Please make these something like

 - star >=2.5.3a, <3
 - bioconductor-biomart >=2.34.2, <3
 - bioconductor-gviz >=1.22.3, <2

sha256: "5dbb97da27f29f53a8282e456ba0093bbdde292eb9e76405434117da4d1a76be"

build:
number: 1
Copy link
Member

Choose a reason for hiding this comment

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

Should be 0 - no packages exist yet.

@@ -0,0 +1,44 @@

Copy link
Member

Choose a reason for hiding this comment

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

Remove empty line

@@ -15,8 +15,6 @@ END
)
# Clinker wrapper - invoke bpipe

#bpipe -p option1="something" -p option2="something_else" [...] $CLINKERDIR/workflow/clinker.pipe /path/to/*.fastq.gz
# parse and accept parameters in bpipe style, then pass on to fixed clinker.pipe location
Copy link
Member

Choose a reason for hiding this comment

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

It appears this entire file should be distributed upstream. Can you create a PR there and link it in the recipe?

Copy link
Contributor

Choose a reason for hiding this comment

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

This recipe has been revisited in #25647 . Though will note here, this is same approach as for the JAFFA recipe with wrapper script seperate from source.

notes: |
Wrapper script provided to indicate clinker is a bpipe pipeline,
provide example command from wiki, and also a passthrough option.
Tests run from /tmp to avoid Docker permission error.
Copy link
Member

Choose a reason for hiding this comment

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

Would you like to add yourself as recipe-maintainers: [username]?

@epruesse epruesse changed the title Clinker Fix (=really add) Clinker Jan 23, 2019
@lonsbio
Copy link
Contributor

lonsbio commented Dec 16, 2020

This can be closed, #25647 is latest update to recipe and fixes other issues

@epruesse epruesse closed this Dec 17, 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.

4 participants