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

samtools stats: remove plot_bamstats call #2579

Merged

Conversation

bernt-matthias
Copy link
Contributor

@bernt-matthias bernt-matthias commented Sep 3, 2019

To get rid of the gnuplot and perl dependency

advantage: no additional mulled environment needed

FOR CONTRIBUTOR:

  • - I have read the CONTRIBUTING.md document and this tool is appropriate for the tools-iuc repo.
  • - License permits unrestricted use (educational + commercial)
  • - This PR adds a new tool or tool collection
  • - This PR updates an existing tool or tool collection
  • - This PR does something else (explain below)

will work after bioconda/bioconda-recipes#17239
this is merged

advantage: no additional mulled environment needed
@bernt-matthias bernt-matthias changed the title remove gnuplot and per dependency samtools stats: remove gnuplot and perl dependency Sep 3, 2019
@bgruening
Copy link
Member

Is the additional mulled env problematic? I think bioconda will not like it. There was recently a discussion about adding R to Picard and this was shut down. bioconda/bioconda-recipes#3859

What you can do is create a samtools-full packages or similar, but then again you have an additional environment.

@bernt-matthias
Copy link
Contributor Author

No real problem, the extra env just takes more memory which is not really necessary.

Interesting discussion. I'm pro adding the extra dependency to the package. Lets see what the community thinks.

After finding out that multiqc can plot the stats I kind of regret that I added the plot_bamstats call... we might as well remove it .. it would not remove any functionality from Galaxy.

@bgruening
Copy link
Member

Thank remove it :)

No real problem, the extra env just takes more memory which is not really necessary.

If you mean storage here, think about it the other way, everyone that just needs samtools, including all our other tools that include samtools, will get now R and gnuplot as well - in their mulled-env.

Sugest to the user to use MultiQC instead.. which is much better.
@bernt-matthias bernt-matthias force-pushed the topic/samtools-perl-gnuplot branch from b119357 to 7cf4774 Compare September 4, 2019 06:36
@bernt-matthias bernt-matthias changed the title samtools stats: remove gnuplot and perl dependency samtools stats: remove plot_bamstats call Sep 4, 2019
@bernt-matthias
Copy link
Contributor Author

Would it be OK to keep the version. For me it would be fine if the change goes to the TS with the next mayor improvement of the tools.

@bgruening
Copy link
Member

Not sure how? Do you mean not merging this PR, or merge and kill travis to not upload the recipes?

@bernt-matthias
Copy link
Contributor Author

Ahh .. lets just bump :) ..

Maybe it would be an option not to upload to the TS if the version is already present in the TS anyway?

@mvdbeek
Copy link
Member

mvdbeek commented Sep 6, 2019

Can you bump the version ? I don't see the point of holding back releases ? The smaller the release the better IMO.

@bernt-matthias
Copy link
Contributor Author

sure. done

@mvdbeek mvdbeek merged commit 558d30f into galaxyproject:master Sep 6, 2019
@bernt-matthias bernt-matthias deleted the topic/samtools-perl-gnuplot branch March 8, 2020 10:39
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