Add tool AMBER #5970

SantaMcCloud · 2024-04-29T01:59:25Z

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)

paulzierep · 2024-04-29T08:03:29Z

tools/amber/amber.xml

+                label="Genome coverages TSV file"
+                help="Input a TSV file where the genome coverage is stated. Look at the help section to se how this file should look like!" />
+        </section>
+        <section name="tox" title="taxonomic binning-specific option">


Can you also allow using it from history, we need a test for it, I think a subsampled file as shown here could work: https://github.com/galaxyproject/tools-iuc/tree/2d380eea5900d7650c5ddfe8cf8d19d2554d0031/tools/recentrifuge/test-data/test-db

We need to point to the nodes.dmp file which is in the path given in: https://github.com/galaxyproject/tools-iuc/blob/2d380eea5900d7650c5ddfe8cf8d19d2554d0031/data_managers/data_manager_fetch_ncbi_taxonomy/tool-data/ncbi_taxonomy.loc.sample

Should be done with a26dba5

paulzierep · 2024-04-29T08:06:27Z

tools/amber/amber.xml

+        <data name="metrics_bin" format="tsv" label="${tool.name}: Bin metrics" />
+    </outputs>
+    <tests>
+        <test expect_num_outputs="4">


this test need an output check

Done with 80dd926

paulzierep · 2024-04-29T08:07:26Z

tools/amber/amber.xml

+                    <param name="keyword" value="circular element" />
+                </conditional>
+            </section>
+            <output name="html">


Can we check for something more meaningful ? Doesn't the tool produce the same output on each run? Then we could check if the files are the same ... ?

The problem is that the HTML file is too big to upload here, but I used the desc option to bring something unique in the file, this was done with 0b53f9d

paulzierep · 2024-04-29T08:10:26Z

tools/amber/amber.xml

+                label="Genome coverages TSV file"
+                help="Input a TSV file where the genome coverage is stated. Look at the help section to se how this file should look like!" />
+        </section>
+        <section name="tox" title="taxonomic binning-specific option">


We need to point to the nodes.dmp file which is in the path given in: https://github.com/galaxyproject/tools-iuc/blob/2d380eea5900d7650c5ddfe8cf8d19d2554d0031/data_managers/data_manager_fetch_ncbi_taxonomy/tool-data/ncbi_taxonomy.loc.sample

bgruening · 2024-04-29T08:45:40Z

tools/amber/amber.xml

@@ -0,0 +1,336 @@
+<tool id="amber" name="AMBER" version="@TOOL_VERSION@+galaxy@VERSION_SUFFIX@" profile="21.05">
+    <description>Evaluation package for the comparative assessment of genome reconstructions and taxonomic assignments from metagenome benchmark datasets</description>


The description is a bit long, can it be shortend?

Doen with 0e60c5f

bgruening · 2024-04-29T08:46:03Z

tools/amber/amber.xml

@@ -0,0 +1,336 @@
+<tool id="amber" name="AMBER" version="@TOOL_VERSION@+galaxy@VERSION_SUFFIX@" profile="21.05">


Should the tool ID and folder name be cami_amber?

Done with 40d1e78 , 54c689b and 9795d98

bgruening · 2024-04-29T08:47:16Z

tools/amber/amber.xml

+        <param argument="--min_length" type="integer" value="" optional="true"
+            label="Mimimum length of sequences"
+            help="Input how long the sequnces has to be" />  
+        <param argument="--stdout" type="boolean" truevalue="--stdout" falsevalue="" checked="false"


How large is this? Should we skip this parameter or enable it by default? Or put it into a real optional output file?

Done with 54c689b

bgruening · 2024-04-29T08:48:00Z

tools/amber/amber.xml

+
+        cp './output/index.html' '$html' &&
+        cp './output/heatmap_bar.png' '$path_to_html' &&
+        cp './output/results.tsv' '$result' &&


instead of copying, moving would be better.

You can completely avoid it by using from_work_dir on the <output>s ...

Done with 2cbb875

bgruening · 2024-04-29T08:52:28Z

tools/amber/amber.xml

+    </inputs>
+    <outputs>
+        <data name="html" format="html" label="${tool.name}: HTML" />
+        <data name="result" format="tsv" label="${tool.name}: Results" />


Suggested change

<data name="result" format="tsv" label="${tool.name}: Results" />

<data name="result" format="tabular" label="${tool.name}: Results" />

Done with 9795d98

bgruening · 2024-04-29T08:52:58Z

tools/amber/amber.xml

+                help="Input a TSV file where the genome coverage is stated. Look at the help section to se how this file should look like!" />
+        </section>
+        <section name="tox" title="taxonomic binning-specific option">
+            <param argument="--ncbi_dir" type="text" value="" optional="true"


The user does not know the path. We need to use the NCBI data table here. Please look at other IUC tools that are using this data table.

Should be done with a26dba5

SantaMcCloud · 2024-04-29T14:14:25Z

Oh the link in the commit should not be there in 80dd926 . I wanted to say that the first test is better now.

bgruening · 2024-04-29T19:53:56Z

tools/cami_amber/cami_amber.xml

+    </requirements>
+    <command detect_errors="exit_code">
+    <![CDATA[
+        mkdir -p output inputs&&


Suggested change

mkdir -p output inputs&&

mkdir -p output inputs &&

bgruening · 2024-04-29T19:55:05Z

tools/cami_amber/cami_amber.xml

+        <param argument="--gold_standard_file" format="tabular" type="data"
+            label="Mapping of contigs or reads"
+            help="Input the gold standard file here so amber know the correct IDs for each contig/read" />
+        <repeat name="label" title="Binning names "


Suggested change

<repeat name="label" title="Binning names "

<repeat name="label" title="Binning names"

@SantaMcCloud what about putting the binning-files into the repeat. And remove the multiple="0".

30acaae

I can do this, but it depends on your answer to my comment here: #5970 (comment)

tools/cami_amber/cami_amber.xml

bgruening · 2024-04-29T20:00:48Z

tools/cami_amber/cami_amber.xml

+        <param argument="--desc" type="text" value=""
+            label="HTML description"
+            help="Enter the HTML page description here" />
+        <section name="genome" title="Genome binning-specific options" >


can we also put this into the main repeat section? I imagine something like

repeat:
- input-file
- binning-name
- min_comp
- max_comp

I can make 2 repeat sections. One with the Files and the names since the names, if you want to input some, have to match the number of files. The min_comp and max_conta can also be put together, but there are not needed for every file, since the count for all when some values are given. But for the second repeat section, it can happen that one param has more thresholds than the other, so does it make sense to combine them?

- input-file - binning-name

-> repeat

- min_comp - max_comp

-> normal param, but a comma seperated list, so extra repeat to create that list for both params (only ones for tool)

3270649
a41eb9c

bgruening · 2024-04-29T20:01:42Z

tools/cami_amber/cami_amber.xml

+            </repeat>
+            <param argument="--remove_genomes" type="data" format="tabular" optional="true" 
+                label="TSV file for genomes to remove"
+                help="Input a TSV file with binid and type in each line. In the help section is an example. WARNING: IF THE LIST CONTAIN ALL GENOME THE PROGRAM WILL FAIL!" />


I don't think we need to add caps letter here :)

tools/cami_amber/cami_amber.xml

…o amber

SantaMcCloud · 2024-05-04T22:55:01Z

Currently, the _add_length.xml isn't working, since I have to access the package location somehow to run the .py file there. Because of this problem, I can not add the convert.xml to have a converter for the biobox format.

bgruening · 2024-05-05T07:23:54Z

Try a combination of which and dirname to get the desired path relative to the main executable. Example here

tools-iuc/tools/cojac/macros.xml

Line 41 in 57fa4dc

DB_PATH="\$(dirname "\$(dirname "\$(which cojac)")")/share/cojac/voc" &&

SantaMcCloud · 2024-05-05T20:05:44Z

@paulzierep when you have time you can play a bit with all tools. If there is something I should change, just let me know :)

@bgruening thank you for the help!! :D

bgruening · 2024-05-06T09:14:41Z

tools/cami_amber/cami_amber_add_length.xml

+        ln -s '${gold_standard_file}' '$gold_standard_file.element_identifier' &&
+        ln -s '${fasta_file}' '$fasta_file.element_identifier' &&
+
+        python "\$(dirname "\$(dirname "\$(which amber.py)")")/lib/python3.11/site-packages/src/utils/add_length_column.py" 


@SantaMcCloud I see now. This trick only works if the scripts are in a neutral place, like /share/ etc ...
It will break on installations with python3.10 or python3.12.

We need to fix the conda package here I think. In the conda package we symlink the lib/python3.11/site-packages/src/utils/add_length_column.py package to /bin and then everyone should be happy.

I see you also PR the recipe. I will delete my PR then :)

Also thank you again for the help!

SantaMcCloud · 2024-05-06T22:35:32Z

Now all should be good!

paulzierep

Initial runs for all tools work and produce correct output. Thank you very much @SantaMcCloud
The taxonomic binning needs a test case.
I need to check which DM we can use for the nodes.dmp file.
The HTML out needs its heatmap

paulzierep · 2024-05-07T07:02:42Z

tools/cami_amber/cami_amber_add_length.xml

@@ -0,0 +1,91 @@
+<tool id="cami_amber" name="CAMI AMBER add lenght column" version="@TOOL_VERSION@+galaxy@VERSION_SUFFIX@" profile="21.05">


the tools need a different id each

paulzierep · 2024-05-07T07:02:49Z

tools/cami_amber/cami_amber_convert_to_biobox.xml

@@ -0,0 +1,193 @@
+<tool id="cami_amber" name="CAMI AMBER convert to biobox" version="@TOOL_VERSION@+galaxy@VERSION_SUFFIX@" profile="21.05">


the tools need a different id each

paulzierep · 2024-05-07T07:14:18Z

tools/cami_amber/cami_amber_add_length.xml

+    ]]>
+    </command>
+    <inputs>
+        <param argument="--gold_standard_file" type="data" format="tabular"


can we add, that this works for any biobox file, but amber only needs seq lengths for the gold standard file ?

paulzierep · 2024-05-07T07:21:57Z

tools/cami_amber/cami_amber_convert_to_biobox.xml

+
+        **Input**
+
+        This tool required 2 inputs:


I guess that is the wrong help ?

paulzierep · 2024-05-07T07:34:22Z

tools/cami_amber/cami_amber_convert_to_biobox.xml

+    ]]>
+    </command>
+    <inputs>
+        <param name="work" type="select" label="How should the tool work?"


I think this needs a better description. Maybe like: Merge binning output files ? If yes all input files will be merged into one biobox output (required input by amber). If no each input will be converted into a separate biobox output file. <- although the tool can do it, I am not sure if this is ever needed for anything -> do you have an idea @SantaMcCloud ?

paulzierep · 2024-05-07T07:39:12Z

tools/cami_amber/cami_amber_convert_to_biobox.xml

+        </param>
+        <conditional name="input">
+            <param name="is_select" type="select" label="Choose how to input the files"
+                help="You can either input the files manually or choose to input a collection">


maybe rather something like you can provide each input seperately or as a collection

tools/cami_amber/cami_amber.xml

paulzierep · 2024-05-07T07:58:14Z

tools/cami_amber/cami_amber.xml

+        <section name="tox" title="taxonomic binning-specific option">
+            <conditional name="ncbi">
+                <param name="is_select" type="select" label="Want to use the NCBI database?"
+                    help="Select yes if you want to use the NCBI database from a data manager">


I think the choice should be first: Do you want to evaluate taxonomic binning ? Then select ncbi file manually or form DM

paulzierep · 2024-05-07T08:03:55Z

tools/cami_amber/cami_amber.xml

+                </assert_contents>
+            </output>
+        </test>
+        <test expect_num_outputs="4">


I think to test weather the taxonomic binning evaluation works we need a test dataset we do not have. Strange that they do not provide it in their test set. But I think you can create a minimal example by manipulating the example from github:

Gold standard:

@Version:0.9.1 @SampleID:gsa @@SEQUENCEID BINID TAXID LENGTH RH|P|C37126 Sample6_89 45202 25096 RH|P|C3274 Sample9_91 32644 10009 RH|P|C26099 1053046 765201 689201 RH|P|C35075 1053046 765201 173282 RH|P|C20873 1053046 765201 339258

Binning1:

@Version:0.9.1 @SampleID:gsa @@SEQUENCEID BINID TAXID LENGTH RH|P|C37126 Sample6_89 32644 25096 RH|P|C3274 Sample9_91 32644 10009 RH|P|C26099 1053046 765201 689201 RH|P|C35075 1053046 765201 173282 RH|P|C20873 1053046 765201 339258

And can you also check weather is also works if we only supply the nodes.dmp file ?

I did check it, and you need all 3 to make it work. I just forgot it to mention it somewhere sorry

To evaluate the taxonomic binning we do also need a test data set that has the TAXID column in the gold standard and the binning file. Then it should produce a html like this one: https://cami-challenge.github.io/AMBER/cami_ii_mouse_gut/
The test here still misses that column.

I used this data but got errors, I found out why now. I did create test_gold.tsv. test_binning.tsv and test_binning2.tsv to create the tox binning, BUT you can only create this tab in the HTML result when running this tool manually with either the download NCBI files or the DM. When using the test-db files, we never get such a output since they are mocked or compromised to fit the 1 MB restriction.

paulzierep · 2024-05-07T08:20:48Z

tools/cami_amber/cami_amber.xml

+        </section>
+    </inputs>
+    <outputs>
+        <data name="html" format="html" from_work_dir="output/index.html" label="${tool.name}: HTML" />


The html output is missing the heatmap_bar.png either the output file needs to be patched to get it from an online source (maybe the amber github), or you need to add seconday files to the html, its a minor thing, but looks much better

I did try to make it, but I forgot a step to implement it correctly. Now it should work after I remember what I did missed when did commit the first version of it

Co-authored-by: paulzierep <paul.zierep@googlemail.com>

…o amber

SantaMcCloud · 2024-05-07T21:09:44Z

@paulzierep for the DM you can use this one https://github.com/galaxyproject/tools-iuc/tree/a95f5b04b1219489a327a622184633a561fe5ac0/data_managers/data_manager_fetch_ncbi_taxonomy

I did try it out with this and it worked fine

bgruening · 2024-05-18T18:08:34Z

tools/cami_amber/cami_amber.xml

+<tool id="cami_amber" name="CAMI AMBER" version="@TOOL_VERSION@+galaxy@VERSION_SUFFIX@" profile="21.05">
+    <description>Evaluation package for MAGs</description>
+    <macros>
+        <token name="@TOOL_VERSION@">2.0.4</token>


@SantaMcCloud can you please move those macros to a separate macro file and import it into your tools. The idea behind macros is to DRY. Don't repeat yourself. requirements can also be moved into this macro file and shared. Same for citations and other stuff that can be shared.

bgruening · 2024-05-20T17:50:18Z

Thanks @SantaMcCloud very nice work!

SantaMcCloud · 2024-05-20T17:52:30Z

Thank you :D

SantaMcCloud added 4 commits April 29, 2024 03:58

Add tool AMBER

c9fbcab

fix typo

9f5ab34

add a little info

c12ded9

fix typo

a1ca020

paulzierep reviewed Apr 29, 2024

View reviewed changes

bgruening reviewed Apr 29, 2024

View reviewed changes

SantaMcCloud added 2 commits April 29, 2024 15:48

fix data manager implication

a26dba5

make test galaxyproject#1 better

80dd926

SantaMcCloud added 7 commits April 29, 2024 16:19

make the html output test a bit unique

0b53f9d

make the desc shorter

0e60c5f

rename amber to cami-amber

40d1e78

remove param stdout since it is not needed

54c689b

remove unnecessarycommands

2cbb875

fix format mistake

9795d98

remove unnecessary commands

92fc9a1

bgruening reviewed Apr 29, 2024

View reviewed changes

tools/cami_amber/cami_amber.xml Outdated Show resolved Hide resolved

Update tools/cami_amber/cami_amber.xml

01e65d6

bgruening reviewed Apr 29, 2024

View reviewed changes

tools/cami_amber/cami_amber.xml Outdated Show resolved Hide resolved

bgruening and others added 6 commits April 29, 2024 22:02

Update tools/cami_amber/cami_amber.xml

21e772a

Merge branch 'amber' of https://github.com/SantaMcCloud/tools-iuc int…

71687e5

…o amber

update cami_amber

30acaae

add option to upload ncbi files manually

7d5aaab

merge min and max repeat to one repeat

3270649

Change bin file input into a repeat together with label option

a41eb9c

SantaMcCloud added 2 commits May 4, 2024 23:05

try to upload add_length and to test it

a267893

fixing lint error

98978c3

SantaMcCloud added 3 commits May 5, 2024 17:38

fix test error and make the tool running

8d86553

maybe now fixed

05baf6e

add convert_to_biobox tool

214dd0d

bgruening reviewed May 6, 2024

View reviewed changes

fix path problems

910e406

paulzierep reviewed May 7, 2024

View reviewed changes

SantaMcCloud and others added 10 commits May 7, 2024 21:53

add heatmap to html

4e0d5c8

change ids

64419fa

change the help from add_lenght

5b37033

change the help section

e4a5c27

better label and help for a param

f1bc8d9

better help for a param

5609f6e

Update tools/cami_amber/cami_amber.xml typo

a010b0e

Co-authored-by: paulzierep <paul.zierep@googlemail.com>

better help for a param

881aeee

add new test

364c9b1

Merge branch 'amber' of https://github.com/SantaMcCloud/tools-iuc int…

2917e13

…o amber

modify some test data

9ee3b7c

bgruening reviewed May 18, 2024

View reviewed changes

add macros and spell check evrey file

9e39a80

bgruening approved these changes May 20, 2024

View reviewed changes

bgruening merged commit 807e74c into galaxyproject:main May 20, 2024
13 checks passed

		@@ -0,0 +1,336 @@
		<tool id="amber" name="AMBER" version="@TOOL_VERSION@+galaxy@VERSION_SUFFIX@" profile="21.05">
		<description>Evaluation package for the comparative assessment of genome reconstructions and taxonomic assignments from metagenome benchmark datasets</description>

	<data name="result" format="tsv" label="${tool.name}: Results" />
	<data name="result" format="tabular" label="${tool.name}: Results" />

	<repeat name="label" title="Binning names "
	<repeat name="label" title="Binning names"

		@@ -0,0 +1,91 @@
		<tool id="cami_amber" name="CAMI AMBER add lenght column" version="@TOOL_VERSION@+galaxy@VERSION_SUFFIX@" profile="21.05">

		@@ -0,0 +1,193 @@
		<tool id="cami_amber" name="CAMI AMBER convert to biobox" version="@TOOL_VERSION@+galaxy@VERSION_SUFFIX@" profile="21.05">

Add tool AMBER #5970

Add tool AMBER #5970

Conversation

SantaMcCloud commented Apr 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SantaMcCloud Apr 29, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SantaMcCloud commented Apr 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SantaMcCloud commented May 4, 2024

bgruening commented May 5, 2024

SantaMcCloud commented May 5, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SantaMcCloud commented May 6, 2024

paulzierep left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SantaMcCloud May 7, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SantaMcCloud May 10, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SantaMcCloud commented May 7, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgruening commented May 20, 2024

SantaMcCloud commented May 20, 2024

SantaMcCloud Apr 29, 2024 •

edited

SantaMcCloud commented May 5, 2024 •

edited

SantaMcCloud May 7, 2024 •

edited

SantaMcCloud May 10, 2024 •

edited

SantaMcCloud commented May 7, 2024 •

edited