-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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 repeatmodeler recipe #19137
Fix repeatmodeler recipe #19137
Conversation
This change should fix the problem with this recipe. |
@BiocondaBot please add label |
Which problem does this PR address? Why do the current tests not catch that problem? Can you add a test that would fail with the current version of the recipe, but would pass with the change proposed in this PR? |
If those tests are hard to write then you should at least verify manually that the patch actually fixes the problem. You can use the |
It has been a while this recipe is not working, it is like an empty shell. The tool seems to run but do not perform anything useful. ie (#9988, Dfam-consortium/RepeatModeler#21, https://www.biostars.org/p/368150/, etc...). The current test just check actually the main tool compile. But RepeatModeler is a pack of several tools coming with RepatModeler plus several tools from external resources. If one of them fail the pipeline doesn't crash it just skip steps. Depending the crash the output could be empty or not, but we get a file (consensi.fa). The thing is to test the whole pipeline we need to include a data set and run RepeatModeler on it, that could run for hours... |
@BiocondaBot please fetch artifacts |
Package(s) built on CircleCI are ready for inspection:
You may also use
Docker image(s) built:
|
@BiocondaBot please fetch artifacts |
Package(s) built on CircleCI are ready for inspection:
You may also use
Docker image(s) built:
|
I have added a test now. The test file is 300kb but I cannot make it smaller. The tool needed a fait amount of data to produce a useful output we can check. A least it will avoid to have the problem we got until now where people were trying to use a recipe that was not really working. |
@bioconda-bot please merge I'm not overly enthused about a 300kb test file, but it looks like this is the only way currently to prevent regression in this recipe. |
I will attempt to upload artifacts and merge this PR. This may take some time, please have patience. |
Thanks for fixing this. Has this updated to RepeatModeler2 yet? |
Not yet... at least not by me. |
I have a bit of a backlog, but updating the RepeatMasker and RepeatModeler recipes is still on my radar. |
Done |
Bioconda requires reviews prior to merging pull-requests (PRs). To facilitate this, once your PR is passing tests and ready to be merged, please add the
please review & merge
label so other members of the bioconda community can have a look at your PR and either make suggestions or merge it.Everyone has access to the following BiocondaBot commands, which can be given in a comment:
@BiocondaBot please update
will cause the BiocondaBot to merge the master branch into a PR@BiocondaBot please add label
will add theplease review & merge
label.@BiocondaBot please fetch artifacts
will post links to packages and docker containers built by the CI system. You can use this to test packages locally before merging.For members of the Bioconda project, the following command is also available:
@BiocondaBot please merge
will cause packages/containers to be uploaded and a PR merged. Someone must approve a PR first! This has the benefit of not wasting CI build time required by manually merging PRs.If you have questions, please post them in gitter or ping
@bioconda/core
in a comment (if you are not able to directly ping@bioconda/core
then the bot will repost your comment and enable pinging).