Sylph tool Wrapper #1518

tcollins2011 · 2024-10-08T20:51:12Z

Moving the sylph tool wrapper pull request to galaxytools due to the metadeata file associated with the .syldb database having a size of 6mb.

… file

tcollins2011 · 2024-10-08T20:51:42Z

@astrovsky01 Connecting you to the PR

bgruening

Thanks @tcollins2011!

The DB is tiny? Is it the wrong DB?
A few initial comments inline.

tools/sylph/sylph.xml

bgruening · 2024-10-09T06:49:20Z

tools/sylph/sylph.xml

+        <expand macro="input_database"/>
+    </inputs>
+    <outputs>
+        <data format="tabular" name="output" label="${tool.name} on ${on_string}" from_work_dir="output.tsv"/>


you could add here as metadata the column names if you like. they seem to be static.

They query and profile commands both generate an output tabular file, but are slightly different (i.e for output_1.tabular and output_5.tabular are similar but slightly different, and only difference is query vs profile). We could split this into two tools if you think that would be valuable, though?

tools/sylph/tool_data_table_conf.xml.sample

tools/sylph/tool_data_table_conf.xml.test

tools/sylph/test-data/test R1.fq

bebatut

Thanks @tcollins2011! I put some comments.

A big one: it seems that both Python scripts come from https://github.com/bluenote-1577/sylph-utils and there is no LICENSE in the repository. So we are not allowed to take the scripts and add them here.
It might be better to find a way (maybe talking with authors) so they make the scripts available via conda maybe. That would avoid duplication

tools/sylph/.shed.yml

tools/sylph/macros.xml

bebatut · 2024-10-09T07:14:46Z

tools/sylph/sylph.xml

+            <param name="functions" type="select" label="ANI querying or taxonomic profiling" help="ANI(Average Nucleotide Identity) is a measure of nucleotide-level similarity between the genomes of two microogranisms. Profile querying involves comparing functional or sequence profiles derived from the genomic data.">
+                <option value="profile">Taxanomic Profile</option>
+                <option value="ani">ANI Query</option>


I am wondering if it would not more sense to have 2 Galaxy tools (one for sylph profile, one for sylph query).
That would be closer of how people would use it via command-line. What do you think?

@tcollins2011 any opinion here?

tools/sylph/sylph.xml

bebatut · 2024-10-09T07:27:50Z

tools/sylph/sylph.xml

+                #else:
+                    && ln -s "$database_select.metadata" "database.tsv.gz"
+                #end if
+                && python "$__tool_directory__/sylph_to_taxprof.py" -s output.tsv -m database.tsv.gz -o metaphlan_


You probably need to put in a # if "metaphlan" in $function.outputs:

The metaphlan tool must be run regardless if krona, metphlan, or both are requested. The krona formatted file is generated from the metaphlan converted file. The #if part is about if we hand it back to the user

tools/sylph/sylph.xml

astrovsky01 · 2024-10-09T11:36:06Z

@bgruening

The DB is tiny? Is it the wrong DB? A few initial comments inline.

The DB we used in this for testing is basically just e. coli, and is as minimal as possible to run tests againstt. Even at that size, though, the metadata file was too big for the repo. That's also why the final test is just checking command text. The next tool we were planning to work on was one to generate sylph db and their associated metada files so users can build their own. In the meantime, I added a readme clarifying how to get the pre-made db and metadata.

tools/sylph/macros.xml

tcollins2011 · 2024-10-09T13:52:59Z

@bgruening @astrovsky01 We could instead wrap a larger database for more thorough testing. We essentially made as small a database as possible when we trying to push this to tools-iuc, but if you think it would provide more thorough testing we could instead swap it out for a larger database. In theory though, the tiny database should allow for fully functional testing within galaxy.

Adding a profile to the tool Co-authored-by: Björn Grüning <[email protected]>

Changing to correct repository owner Co-authored-by: Bérénice Batut <[email protected]>

Improving description of tool Co-authored-by: Bérénice Batut <[email protected]>

Improved description of the tool Co-authored-by: Bérénice Batut <[email protected]>

bgruening · 2024-10-09T14:01:38Z

@bgruening @astrovsky01 We could instead wrap a larger database for more thorough testing. We essentially made as small a database as possible when we trying to push this to tools-iuc, but if you think it would provide more thorough testing we could instead swap it out for a larger database. In theory though, the tiny database should allow for fully functional testing within galaxy.

All good, I was just confused because you said the DB is 6MB and I wanted to check if the submitted file was a mistake.

Updated due to publication Co-authored-by: Bérénice Batut <[email protected]>

astrovsky01 · 2024-10-09T15:35:46Z

it seems that both Python scripts come from https://github.com/bluenote-1577/sylph-utils and there is no LICENSE in the repository.

@bebatut the krona converter is actually a modified version of the one wrapped in the Metaphlan tool in galaxy. I believe it's from Galaxy, too, since I don't see it in the metaphlan original repo

Co-authored-by: Bérénice Batut <[email protected]>

…for security

Co-authored-by: Bérénice Batut <[email protected]>

bgruening · 2024-10-10T05:56:30Z

It looks like test files are missing and the limiting is not passing.

tcollins2011 · 2024-10-10T14:26:23Z

@bgruening Alright I think everything should be passing now except the file size check.

bgruening · 2024-10-10T14:44:56Z

tools/sylph/sylph.xml

+                <param name="functions" value="profile"/>
+                <param name="min_num_kmers" value="49"/>
+            </conditional>
+            <output name="output" value="output_2.tabular" compare="sim_size"/>


please don't use sim_size on text outputs, use asserts :)

bgruening · 2024-10-10T14:47:57Z

tools/sylph/macros.xml

+            <when value="cached">
+                <param label="Select a sylph database" name="sylph_database" type="select">
+                    <options from_data_table="sylph_databases">
+                        <validator message="No Sylph databases are available" type="no_options" />


add here a filter for version 1 or something like that ... and then we include version 1 in the test file.

Whenever the tool changes to DB layout we increase this version.

ping, you are not filtering the DB here according to the version

bgruening

Looks mostly good to me, just 2 small comments.

….xml file to have more shared logic

tcollins2011 · 2024-10-25T17:31:10Z

@bgruening Sylph has been split into two separate tools now both within this directory. Additionally, I see that we are still failing the file size check, but I'm not sure if it is possible to reduce the database metadata files to a smaller size.

bgruening

Thanks @tcollins2011! Added a few more smaller comments!

bgruening · 2024-10-26T13:19:11Z

tools/sylph/macros.xml

+    <token name="@LICENSE@">MIT</token>
+    <token name="@DB_SELECTOR@"><![CDATA[
+        #if $database_select.select == 'cached':
+        ln -s '$database_select.sylph_database.fields.path/database.syldb' 'database.syldb' &&


Suggested change

ln -s '$database_select.sylph_database.fields.path/database.syldb' 'database.syldb' &&

ln -s '$database_select.sylph_database.fields.path/database.syldb' 'database.syldb' &&

bgruening · 2024-10-26T13:19:31Z

tools/sylph/macros.xml

+    ]]></token>
+    <token name="@SINGLE_INPUT@"><![CDATA[
+        #if $sketch.input.ext == 'fastqsanger'
+        #set $ext = 'fastq'


Suggested change

#set $ext = 'fastq'

#set $ext = 'fastq'

bgruening · 2024-10-26T13:22:02Z

tools/sylph/macros.xml

+            <requirement type="package" version="3.11.9">python</requirement>
+            <requirement type="package" version="2.2.3">pandas</requirement>


Suggested change

<requirement type="package" version="3.11.9">python</requirement>

<requirement type="package" version="2.2.3">pandas</requirement>

<requirement type="package" version="3.11">python</requirement>

<requirement type="package" version="2.2">pandas</requirement>

bgruening · 2024-10-26T13:22:34Z

tools/sylph/macros.xml

+    </xml>
+    <xml name="creator">
+        <creator>
+            <organization name="University of Toronto" url="https://github.com/bluenote-1577/sylph"/>


This is meant to acknowledge you, the Galaxy tool creator.

bgruening · 2024-10-26T13:25:51Z

tools/sylph/test-data/sylph_databases.loc

@@ -0,0 +1 @@
+sylph_db	sylph_db	${__HERE__}/sylph_db	month,year   


month,year should be something .. here isn' it?

It could be simply 1

bgruening · 2024-10-26T13:30:49Z

tools/sylph/macros.xml

+            <when value="cached">
+                <param label="Select a sylph database" name="sylph_database" type="select">
+                    <options from_data_table="sylph_databases">
+                        <validator message="No Sylph databases are available" type="no_options" />


ping, you are not filtering the DB here according to the version

bgruening · 2024-10-26T13:31:30Z

tools/sylph/sylph_query.xml

+        -t \${GALAXY_SLOTS:-4}
+        database.syldb
+        sylph_sketches/*.sylsp
+        -o $output


Suggested change

-o $output

-o '$output`

Please check if the rest is also quoted

bgruening · 2024-10-26T13:32:28Z

tools/sylph/sylph_query.xml

+            </param>
+        <!-- Only permitting fastq as tool input only allows fastq and fastq.gz as file ext -->
+            <when value="single">
+                <param name="input" type="data" format="fastq,fastq.gz,fastqsanger,fastqsanger.gz" label="Single-end input reads"/>


Why is this needed, can we not just go with the multiple=true option?

bgruening · 2024-10-26T13:33:18Z

tools/sylph/sylph_profile.xml

+                <option value="single">Individual single-end reads</option>
+                <option value="single_group">Group of single-ended reads</option>
+                <option value="paired">One set of paired-end reads</option>
+                <option value="paired_group"> Group of paired-ended reads</option>


Suggested change

<option value="paired_group"> Group of paired-ended reads</option>

<option value="paired_group">Group of paired-ended reads</option>

bgruening · 2024-10-26T13:34:20Z

tools/sylph/sylph_profile.xml

@@ -0,0 +1,210 @@
+<tool id="sylph_profile" name="sylph profile" version="@TOOL_VERSION@+galaxy@VERSION_SUFFIX@" profile="23.0">
+    <expand macro='description'/>


shouldn't this be specific to this tool? (and the other)

transfered sylph to galaxytools due to size of test database metadata…

027c8be

… file

bgruening reviewed Oct 9, 2024

View reviewed changes

bebatut reviewed Oct 9, 2024

View reviewed changes

tools/sylph/macros.xml Outdated Show resolved Hide resolved

tcollins2011 and others added 4 commits October 9, 2024 09:53

Update tools/sylph/sylph.xml

d7fc0e7

Adding a profile to the tool Co-authored-by: Björn Grüning <[email protected]>

Update tools/sylph/.shed.yml

6146fb0

Changing to correct repository owner Co-authored-by: Bérénice Batut <[email protected]>

Update tools/sylph/.shed.yml

f1e4551

Improving description of tool Co-authored-by: Bérénice Batut <[email protected]>

Update tools/sylph/macros.xml

6b9d9cb

Improved description of the tool Co-authored-by: Bérénice Batut <[email protected]>

Update tools/sylph/macros.xml

55d54f1

Updated due to publication Co-authored-by: Bérénice Batut <[email protected]>

tcollins2011 and others added 5 commits October 9, 2024 13:10

Update tools/sylph/.shed.yml

3519a0b

Co-authored-by: Bérénice Batut <[email protected]>

replaced all double quotes in the command section with single quotes …

8944a8d

…for security

Update tools/sylph/sylph.xml

63ca10f

Co-authored-by: Bérénice Batut <[email protected]>

updated database sylmlink to better reflect the name

46e5d68

changed ouput name

477e325

tcollins2011 and others added 6 commits October 10, 2024 09:24

changed database path names and sample file

25d9bac

python linting and tab spacing

1513faf

Merge branch 'master' into sylph

691a915

fixing flake8 linting problems

9221bb8

force adding the extra test files and fixing a spacing issue in python

d86bcf0

actually remembring to add the correct whitespace file to my commit

3c35f6f

bgruening reviewed Oct 10, 2024

View reviewed changes

changed profile and query to be different tools and upated the macros…

5566c61

….xml file to have more shared logic

bgruening reviewed Oct 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sylph tool Wrapper #1518

Sylph tool Wrapper #1518

tcollins2011 commented Oct 8, 2024

tcollins2011 commented Oct 8, 2024

bgruening left a comment

bgruening Oct 9, 2024

astrovsky01 Oct 10, 2024

bebatut left a comment

bebatut Oct 9, 2024

bgruening Oct 10, 2024

bebatut Oct 9, 2024

astrovsky01 Oct 10, 2024

astrovsky01 commented Oct 9, 2024 •

edited

Loading

tcollins2011 commented Oct 9, 2024

bgruening commented Oct 9, 2024

astrovsky01 commented Oct 9, 2024

bgruening commented Oct 10, 2024

tcollins2011 commented Oct 10, 2024

bgruening Oct 10, 2024

bgruening Oct 10, 2024

bgruening Oct 26, 2024

bgruening left a comment

tcollins2011 commented Oct 25, 2024

bgruening left a comment

bgruening Oct 26, 2024

bgruening Oct 26, 2024

bgruening Oct 26, 2024

bgruening Oct 26, 2024

bgruening Oct 26, 2024

bgruening Oct 26, 2024

bgruening Oct 26, 2024

bgruening Oct 26, 2024

bgruening Oct 26, 2024

bgruening Oct 26, 2024

	ln -s '$database_select.sylph_database.fields.path/database.syldb' 'database.syldb' &&
	ln -s '$database_select.sylph_database.fields.path/database.syldb' 'database.syldb' &&

		<requirement type="package" version="3.11.9">python</requirement>
		<requirement type="package" version="2.2.3">pandas</requirement>

		@@ -0,0 +1 @@
		sylph_db sylph_db ${__HERE__}/sylph_db month,year

	<option value="paired_group"> Group of paired-ended reads</option>
	<option value="paired_group">Group of paired-ended reads</option>

		@@ -0,0 +1,210 @@
		<tool id="sylph_profile" name="sylph profile" version="@TOOL_VERSION@+galaxy@VERSION_SUFFIX@" profile="23.0">
		<expand macro='description'/>

Sylph tool Wrapper #1518

Are you sure you want to change the base?

Sylph tool Wrapper #1518

Conversation

tcollins2011 commented Oct 8, 2024

tcollins2011 commented Oct 8, 2024

bgruening left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bebatut 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

astrovsky01 commented Oct 9, 2024 • edited Loading

tcollins2011 commented Oct 9, 2024

bgruening commented Oct 9, 2024

astrovsky01 commented Oct 9, 2024

bgruening commented Oct 10, 2024

tcollins2011 commented Oct 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgruening left a comment

Choose a reason for hiding this comment

tcollins2011 commented Oct 25, 2024

bgruening 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

astrovsky01 commented Oct 9, 2024 •

edited

Loading