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

newick display: add imagemagick dependency #2710

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 9 additions & 10 deletions tools/newick_utils/newick_display.xml
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
<tool id="newick_display" name="Newick Display" version="1.6">
<tool id="newick_display" name="Newick Display" version="1.6+galaxy1">
<description>visualize a phylogenetic tree</description>
<edam_operations>
<edam_operation>operation_0567</edam_operation>
</edam_operations>
<requirements>
<requirement type="package" version="1.6">newick_utils</requirement>
<requirement type="package" version="7.0.9_6">imagemagick</requirement>
</requirements>
<command detect_errors="aggressive"><![CDATA[
nw_display
Expand Down Expand Up @@ -36,15 +37,13 @@ nw_display
-w $width
$radial

'$fileNewick' > output.svg

#if $outformat == 'png':
Copy link
Member

Choose a reason for hiding this comment

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

Is that maybe also due to the bug fixed by galaxyproject/galaxy#9052 ? Because this would just move output.svg '$output' instead of running the conversion ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope. convert was just not installed. The container test output was convert: command not found.

I tried on my local system to use convert and gm convert and both were not able to create a png from the svg.

Copy link
Member

Choose a reason for hiding this comment

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

I was able to add imagemagick from conda-forge and got a png with this patch:

diff --git a/tools/newick_utils/newick_display.xml b/tools/newick_utils/newick_display.xml
index fbe029b5e..5dcc7b17a 100644
--- a/tools/newick_utils/newick_display.xml
+++ b/tools/newick_utils/newick_display.xml
@@ -5,6 +5,7 @@
     </edam_operations>
     <requirements>
         <requirement type="package" version="1.6">newick_utils</requirement>
+        <requirement type="package" version="7.0.9_6">imagemagick</requirement>
     </requirements>
     <command detect_errors="aggressive"><![CDATA[
 nw_display

But the png seems to miss fonts ?
tree

Copy link
Member

@mvdbeek mvdbeek Nov 30, 2019

Choose a reason for hiding this comment

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

Cool, we can add a minimal fonts package like this to fix it:

diff --git a/tools/newick_utils/newick_display.xml b/tools/newick_utils/newick_display.xml
index fbe029b5e..c75f8685c 100644
--- a/tools/newick_utils/newick_display.xml
+++ b/tools/newick_utils/newick_display.xml
@@ -5,6 +5,8 @@
     </edam_operations>
     <requirements>
         <requirement type="package" version="1.6">newick_utils</requirement>
+        <requirement type="package" version="7.0.9_6">imagemagick</requirement>
+        <requirement type="package" version="1">fonts-conda-forge</requirement>
     </requirements>
     <command detect_errors="aggressive"><![CDATA[
 nw_display
@@ -110,7 +112,7 @@ nw_display
             <param name="radial" value="-r"/>
             <param name="branchlength" value="true"/>
             <param name="outformat" value="png"/>
-            <output name="output" file="tree.png" ftype="png" compare="sim_size" delta="15000"/>
+            <output name="output" file="tree.png" ftype="png" compare="sim_size" delta="18000"/>
         </test>
          <test><!-- test with txt output format -->
             <param name="fileNewick" value="tree.nwk"/>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

great. woudn't it be better to add the fonts package as a requirement to the imagemagick recipe?

Copy link
Member

Choose a reason for hiding this comment

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

Possibly, I'm not sure how that decision is made. It seems imagemagick will use whatever is available.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lets see conda-forge/imagemagick-feedstock#101 .. I will restore the original code.

Copy link
Member

Choose a reason for hiding this comment

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

out of interest ... does graphicsmagick work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gm gives the following error gm convert: invalid primitive argument (small).

&& convert output.svg output.png
&& mv output.png '$output'
#else:
&& mv output.svg '$output'
#end if
'$fileNewick'

#if $outformat == 'png':
| convert - 'png:$output'
#else:
> '$output'
#end if
]]></command>
<inputs>
<param name="fileNewick" format="txt,newick,nw,nwk,nhx,mothur.tre" type="data" label="Newick file"/>
Expand Down Expand Up @@ -112,7 +111,7 @@ nw_display
<param name="outformat" value="png"/>
<output name="output" file="tree.png" ftype="png" compare="sim_size" delta="15000"/>
</test>
<test><!-- test with txt output format -->
<test><!-- test with txt output format -->
<param name="fileNewick" value="tree.nwk"/>
<param name="outformat" value="txt"/>
<output name="output" file="tree.txt" ftype="txt"/>
Expand Down