hifiasm2 allow excluding --primary #6484
Conversation
the galaxy tool forced --primary even though it isn't on by the software's default setting. It also prevented from removing it. This patch allows users to use the hifiasm2 default behaviour of excluding --primary. Patch sets as true to maintain the galaxy default.
|
i honestly don't know how! (i have two patch pull requests, i kept them separate so they can be considered independently) |
i think that's correct? never done it before
There was a problem hiding this comment.
I would also suggest to have only one PR, i.e. include #6483 here. Alteratively do not increase the version suffix in one of them and skip CI?
tools/hifiasm/hifiasm.xml
Outdated
| @@ -216,6 +218,7 @@ | |||
| </param> | |||
| <when value="blank"/> | |||
| <when value="set"> | |||
| <param name="primary" type="boolean" truevalue="--primary" falsevalue="" checked="true" label="Create primary and alternate assemblies" help="Hifiasm outputs a primary assembly and two balanced haplotypes by default. This will output a primary assembly and alternate assembly only (--primary)."/> | |||
There was a problem hiding this comment.
| <param name="primary" type="boolean" truevalue="--primary" falsevalue="" checked="true" label="Create primary and alternate assemblies" help="Hifiasm outputs a primary assembly and two balanced haplotypes by default. This will output a primary assembly and alternate assembly only (--primary)."/> | |
| <param argument="--primary" type="boolean" truevalue="--primary" falsevalue="" checked="true" label="Create primary and alternate assemblies" help="Hifiasm outputs a primary assembly and two balanced haplotypes by default. This will output a primary assembly and alternate assembly only"/> |
integrates patch-1 of galaxyproject#6483; adds --primary and correctly handles output files; left comment on whether argument= or name= ought to be used. can write tests if code review is +ve
|
fixed lint errors. can't find why it complains about <when>
removed remnant from prev commit
|
no idea why lint fails for |
ok it was my comment. fixed and provide it here: |
There was a problem hiding this comment.
3. left comment on whether argument= or name= ought to be used. please advise
Argument would be preferred: https://galaxy-iuc-standards.readthedocs.io/en/latest/best_practices/tool_xml.html#parameter-name-argument-and-help. It will create a variable with the same name as before (i.e. dashes are stripped automatically) and add the argument in parentheses to the end of the help (which is then nicely consistent between tools).
| @@ -320,23 +327,37 @@ | |||
| </inputs> | |||
| <outputs> | |||
| <!--Standard mode--> | |||
| <data name="raw_unitigs" format="gfa1" from_work_dir="output.r_utg.gfa" label="${tool.name} on ${on_string}: haplotype-resolved raw unitig graph for pseudohaplotype assembly"> | |||
| <filter>mode['mode_selector'] == 'standard' and hic_partition['hic_partition_selector'] == 'blank'</filter> | |||
| <data name="raw_unitigs" format="gfa1" from_work_dir="*.r_utg.gfa" label="${tool.name} on ${on_string}: haplotype-resolved raw unitig graph for pseudohaplotype assembly"> | |||
There was a problem hiding this comment.
Wondering if the *.r_utg.gfa is correct here. Seems to conflict with:
<data name="raw_unitigs_trio" format="gfa1" from_work_dir="output.dip.r_utg.gfa"...>
The prefix should be fix according to the docs. So I guess this should be reverted to output.r_utg.gfa
| <data name="raw_unitigs" format="gfa1" from_work_dir="output.r_utg.gfa" label="${tool.name} on ${on_string}: haplotype-resolved raw unitig graph for pseudohaplotype assembly"> | ||
| <filter>mode['mode_selector'] == 'standard' and hic_partition['hic_partition_selector'] == 'blank'</filter> | ||
| <data name="raw_unitigs" format="gfa1" from_work_dir="*.r_utg.gfa" label="${tool.name} on ${on_string}: haplotype-resolved raw unitig graph for pseudohaplotype assembly"> | ||
| <filter>mode['mode_selector'] == 'standard' and hic_partition['hic_partition_selector'] == 'blank'</filter> |
There was a problem hiding this comment.
Just an idea: If you have a filter <filter>A and B</filter>. Then you could rewrite this as:
<filter>A</filter>
<filter>B</filter>
Might make all this more readable.
| <data name="primary_contig_graph" format="gfa1" from_work_dir="output.p_ctg.gfa" label="${tool.name} on ${on_string}: primary assembly contig graph for pseudohaplotype assembly"> | ||
| <filter>mode['mode_selector'] == 'standard' and hic_partition['hic_partition_selector'] == 'blank'</filter> | ||
| <filter>mode['mode_selector'] == 'standard' and hic_partition['hic_partition_selector'] == 'blank' and (assembly_options['assembly_selector'] == 'blank' or (assembly_options['assembly_selector'] == 'set' and assembly_options['primary']))</filter> |
There was a problem hiding this comment.
So what you are doing here is to state that output.p_ctg.gfa is produced only if --primary is set (which is the default). According to the docs it seems to be even slightly more complicated (but I'm not sure if it really needs to be done here :) ).
--primary
Output a primary assembly and an alternate assembly. Enable this option or -l0 outputs a primary assembly and an alternate assembly.
It appears to me that analogous changes should be done for all outputs. But feel free not to do so in this PR.
There was a problem hiding this comment.
would you read this doc to mean that -l0 is the same as --primary? or does it mean that you should use --primary or ("otherwise") -l0 will output a primary and alt assembly?
i think the former (-l0 or --primary produce primary and alt assemblies)?
| </data> | ||
|
|
||
| <!--Primary off mode--> | ||
| <data name="main_contig_graph" format="gfa1" from_work_dir="output.bp.p_ctg.gfa" label="${tool.name} on ${on_string}: main assembly contig graph for pseudohaplotype assembly"> |
There was a problem hiding this comment.
The three new outputs should be covered by tests.
Do you know what the statement from the docs means: "Hifiasm generates the following assembly graphs only with HiFi reads in default:" Can we check this in the filters?
Overall I would like to see all outputs covered by at least one test (but again, feel free not to do so here).
There was a problem hiding this comment.
i'm not sure but every time i use it, that output file is identical to the haplo1 output. the tests will help to figure it out
i can do the tests, just need to find a bit of time to learn how as i 've never done it before. hopefully next week!
There was a problem hiding this comment.
@alpapan please let us know if you need any help
the galaxy tool forced --primary even though it isn't on by the software's default setting. It also prevented from removing it. This patch allows users to use the hifiasm2 default behaviour of excluding --primary. Patch sets as true to maintain the galaxy default.
FOR CONTRIBUTOR: