hifiasm: fix --primary output, --hg-size decimals, test typos, modern profile#7985
Open
alpapan wants to merge 3 commits into
Open
hifiasm: fix --primary output, --hg-size decimals, test typos, modern profile#7985alpapan wants to merge 3 commits into
alpapan wants to merge 3 commits into
Conversation
… profile * Fix duplicate output names causing primary_contig_graph to disappear when --primary is enabled. The output was declared multiple times under the same name, so Galaxy registered only the last (filter-disabled) one. The "Primary off mode" primary contig output is now named bp_primary_contig_graph; hap1/hap2 std labels are suffixed "(balanced haplotype)" to disambiguate from the trio outputs. * Wire the --primary toggle correctly: drop the unconditional --primary in the command line and fix the broken nested cheetah #if so the user-facing checkbox actually controls the flag. * --hg-size: accept decimal values (e.g. 1.3g). The sanitizer was stripping the dot, turning 1.3g into 13g silently. Added "." to the valid character set and tightened the validator regex. * Tests: wrap reads/mode_selector in <conditional name="mode"> so tests validate under modern Galaxy tool profiles; fix max_kooc -> max_kocc (Test 9) and hom-cov -> hom_cov (Test 11) typos; fix Test 5 trio param placement (reads at trio level, trio_input_selector inside trioinput). * Add profile="23.0" (was implicitly the legacy 16.01 profile) and bump VERSION_SUFFIX to 4.
bgruening
reviewed
May 13, 2026
RZ9082
reviewed
May 13, 2026
Member
RZ9082
left a comment
There was a problem hiding this comment.
Could you also please remove the .lint_skip file, and fix the linting errors if there are any?
* Use @Profile@ token (25.0) instead of hardcoded profile attr. * Replace `#if $assembly_options.primary: --primary #end if` with direct `$assembly_options.primary` substitution (canonical Galaxy idiom for booleans with truevalue/falsevalue). * Switch `name="primary"` to `argument="--primary"` so the param identifier auto-derives from the CLI flag; help text trimmed. * Fix test 17: `expect_num_outputs` 5 -> 6 (the noseq_files collection is counted), and switch the hap2 content assertion from `^S` regex to `has_size value="0"` since hifiasm emits an empty hap2 GFA for the tiny hifiasm-in1.fa.gz fixture. * Remove tools/hifiasm/.lint_skip; planemo lint is now clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bgruening
reviewed
May 15, 2026
| </output> | ||
| <output name="hap2_contigs_std" ftype="gfa1"> | ||
| <assert_contents> | ||
| <has_size value="0"/> |
bgruening
reviewed
May 15, 2026
| </assert_command> | ||
| <output name="bp_primary_contig_graph" ftype="gfa1"> | ||
| <assert_contents> | ||
| <has_text_matching expression="^S"/> |
Member
There was a problem hiding this comment.
is there just nothing included? Or why is this test so flax/loose?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A handful of bug fixes and test/profile cleanup in
tools/hifiasm/hifiasm.xml.Bug fixes
primary_contig_graphdisappears when the--primarycheckbox is set. Three<data>elements share the nameprimary_contig_graph; Galaxy only registers the last, whose filter requires the opposite checkbox state. The Primary-off primary contig output is now namedbp_primary_contig_graph, and thehap1_contigs_std/hap2_contigs_stdlabels are suffixed with "(balanced haplotype)" to disambiguate from the trio outputs (resolves aOutputsLabelDuplicatedFilterwarning).--primarytoggle was a no-op. The command section had an unconditional--primaryand a broken nested#if(the'blank'branch was inside the'set'branch and could never fire). Reorganised so the user-facing checkbox actually controls the flag while preserving "primary on" as the default.--hg-sizesilently drops decimal points. Entering1.3gwas sanitised to13g. Added.to the sanitizer's valid set and tightened the validator regex to^[0-9]+(\.[0-9]+)?[kKmMGg]?$. Help text updated.Tests / lint
<param name="reads">+<param name="mode_selector">inside<conditional name="mode">for the standard / ONT tests, so they validate under modern Galaxy tool profile versions (resolves 15×TestsCaseValidation).readsmoved to trio level;trio_input_selectormoved insidetrioinputto match the actual input structure.max_kooc→max_kocc(resolvesTestsParamInInputsERROR).hom-cov→hom_cov(param name is derived fromargument="--hom-cov", hyphens become underscores).--primaryOFF path (balanced haplotype outputs).profile="23.0"(was implicitly legacy16.01) and bumpVERSION_SUFFIXto4.Test plan
planemo lint— clean (no warnings, no errors).planemo testagainst existing test data.--hg-sizeto1.3gand confirm the command line carries1.3g, not13g.