Skip to content

Conversation

nromashchenko
Copy link

@nromashchenko nromashchenko commented Sep 30, 2025

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)

As the title says, this request adds read2tree.

Copy link
Contributor

@bernt-matthias bernt-matthias left a comment

Choose a reason for hiding this comment

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

Thanks for the submission. I added a few comments.


remote_repository_url: https://github.com/galaxyproject/tools-iuc/tree/main/tools/read2tree
categories:
- "Genomics Analysis" No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to use one of the categories of the toolshed.

@@ -0,0 +1,107 @@
<tool id="read2tree" name="Read2Tree" version="2.0.1" python_template_version="3.5">
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use a TOOL_VERSION macro for the version? Like here: https://github.com/galaxyproject/tools-iuc/blob/main/tools/berokka/berokka.xml

Comment on lines +13 to +14
rm -rf ./genes_dir;
rm -rf ./output;
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not be needed. Every jobs gets a fresh working dir.

Comment on lines +18 to +20
#set $i = 0
#for $f in $marker_genes
#set $i = $i + 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#set $i = 0
#for $f in $marker_genes
#set $i = $i + 1
#for $i, $f in enumerate($marker_genes)

#set $i = 0
#for $f in $marker_genes
#set $i = $i + 1
ln -s -- `readlink -f $f` "./genes_dir/$i".fa;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ln -s -- `readlink -f $f` "./genes_dir/$i".fa;
ln -s '$f' "./genes_dir/$i".fa;

]]></command>

<inputs>
<param name="reads" type="data" multiple="true" format="fasta,fastq" label="Input reads in FASTA or FASTQ"
Copy link
Contributor

Choose a reason for hiding this comment

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

Use fastqsanger and/or fastqillumina instead of fastq.

Does the software accept gzipped data?


<!--param name="marker_genes" type="data" multiple="true" format="fasta" label="Marker genes"/-->

<param name="marker_genes" type="data_collection" collection_type="list"
Copy link
Contributor

Choose a reason for hiding this comment

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

You could also use a data parameter with multiple="true". Users can still use collections. But there might be good reason for using a acollection?

Copy link
Author

Choose a reason for hiding this comment

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

marker genes could be dozens or more files. I figured it's easier to upload them and build into a collection first than do multiple="true". Otherwise on the tool page the user has to select all them one by one again

Copy link
Contributor

Choose a reason for hiding this comment

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

If you look at the tool form there are a few buttons left of the data selector (for data inputs with multiple="true")

  1. Multiple datasets: the user can select multiple datasets
  2. Dataset collection the user can select a collection

That is collection input also works in this case. The only downside is that the element_identifiers are not really useful for users choosing option 1.

I'm completely fine with collection input. Just wanted to make you aware.


<tests>
<test>
<param name="reads" value="sample_1.fastq,sample_2.fastq"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Guess the fastq files could be downsampled to reduce size?

<param name="marker_genes" type="data_collection" collection_type="list"
label="Marker gene files" help="A set of reference orthologous groups, i.e. marker genes. See https://github.com/DessimozLab/read2tree for detail."/>

<param name="dna_ref" type="data" format="fasta" label="DNA reference"
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to be optional? Could you add a few words to the help when this is needed and what it is?

@@ -0,0 +1,107 @@
<tool id="read2tree" name="Read2Tree" version="2.0.1" python_template_version="3.5">
Copy link
Contributor

Choose a reason for hiding this comment

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

Add profile="24.0"

<param name="marker_genes" type="data_collection" collection_type="list"
label="Marker gene files" help="A set of reference orthologous groups, i.e. marker genes. See https://github.com/DessimozLab/read2tree for detail."/>

<param name="dna_ref" type="data" format="fasta" label="DNA reference"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use the argument attribute instead of (or in addition to) name

</param>
<param name="dna_ref" value="dna_ref.fa"/>
<output name="output_tree">
<assert_contents>
Copy link
Member

Choose a reason for hiding this comment

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

maybe making this test a bit more stricter? For example by asserting the line count?

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