Skip to content

feat(ENGKNOW-3940): add VARNORM_WITH_BUILD command - #145

Draft
gmagnu wants to merge 2 commits into
mainfrom
ENGKNOW-3940-gor-varnorm-with-build
Draft

gmagnu wants to merge 2 commits into
mainfrom
ENGKNOW-3940-gor-varnorm-with-build

Conversation

@gmagnu

@gmagnu gmagnu commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Jira: ENGKNOW-3940

Problem

VARNORM always normalizes against the project default build (VarNormAnalysis hard-codes createRefSeq()). The
liftover templates in gdb-cla-queries normalize after lifting, so when the target build is not the project default
(e.g. hg38tohg19 in an hg38 project) indels are aligned against the wrong reference.

Change

New command, same options as VARNORM:

gor ... | VARNORM_WITH_BUILD build refcol altcol [-seg] [-left|-right] [-trim] [-span N]
  • build is a chromSeq folder, quoted or unquoted, resolved like refbases_with_build (ProjectContext.createRefSeq(path)).
  • VarNormAnalysis gets an optional refSeqPath (default None, so existing callers are unchanged). The RefSeq is created once per analysis and closed on finish.
  • VARNORM option parsing is moved to VarNorm.parse and shared, not copied.
  • Missing build fails at parse time (Reference build <path> does not exist). Without the check, RefSeqFromChromSeq logs a warning and returns N, so a typo in the path would silently leave variants unnormalized.
  • Docs: VARNORM_WITH_BUILD.rst, command index, lexer.

Review note

The existence check uses getFileReader.unsecure(), because that is the reader RefSeqFromChromSeq reads the build with.
With the secure reader, absolute build paths (e.g. /private/gorkube-mount/...) were rejected in server mode even though
the read itself works. This exposes no more than refbases_with_build already can.

Tests

  • UTestVarNormWithBuild (real ref_mini data):
    • left shift in the chr1 TAACCC repeat
    • quoted path
    • identical to varnorm when the build equals the configured default, for '', -left, -right, -trim, -right -trim
    • overrides the project default
    • absolute path in server mode
    • clear error for a missing build
  • UTestVarnormAnalysis: refSeqPath uses createRefSeq(path), never the default, and closes it.
  • UTestCommandParsing: argument and option cases.
  • Full :gortools:test: 2605 tests, 0 failures, 0 errors.

Follow-up: ENGKNOW-3941 switches the gdb-cla-queries liftover templates to this command.

🤖 Generated with Claude Code

VARNORM always normalizes against the project default reference build.
VARNORM_WITH_BUILD takes a chromSeq path as its first argument (quoted or
unquoted, like refbases_with_build) and normalizes against that build, with
the same options as VARNORM. This lets liftover pipelines normalize lifted
variants against the target build.

- VarNormAnalysis takes an optional refSeqPath; the RefSeq is created once
  per analysis and closed on finish.
- VARNORM option parsing moved to VarNorm.parse and shared by both commands.
- A missing build fails at parse time instead of silently reading as N.
  The check uses the unsecure reader, which is what RefSeqFromChromSeq reads
  with, so absolute build paths work in server mode as for refbases_with_build.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Junit Tests - Summary

4 868 tests  +17   4 697 ✅ +18   19m 39s ⏱️ - 1m 33s
  506 suites + 1     171 💤  -  1 
  506 files   + 1       0 ❌ ± 0 

Results for commit 9179291. ± Comparison against base commit 9536c2f.

♻️ This comment has been updated with latest results.

…osome files

The build folder existence check failed for S3/OCI folders without a trailing
slash and was meaningless on Azure. Probe <build>/chr1.txt or <build>/1.txt
instead, the way RefSeqFromChromSeq reads the files, and name the probed files
in the error. Explain the unsecure() reader, add tests for insertions, right
shift, multiple contigs, contig start and -seg, assert exact output without a
default build, and document options, chromosome naming and example paths.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
// Object stores (S3, OCI) report a folder without a trailing slash as missing, while the chromosome
// files RefSeqFromChromSeq reads (<build>/<chrom>.txt) are there. Only the files may be probed.
String build = "s3://bucket/ref/chromSeq";
Set<String> existing = Set.of(build + "/chr1.txt");
@Test
public void testBuildValidationAcceptsChromosomeNamesWithoutChrPrefix() {
String build = "s3://bucket/ref/chromSeq";
Set<String> existing = Set.of(build + "/1.txt");
public void testBuildValidationRejectsFolderWithoutChromosomeFiles() {
String build = "s3://bucket/ref/chromSeq";
// Even if the folder itself "exists" (Azure always says so), no chromosome file means no usable build.
Set<String> existing = Set.of(build, build + "/");

This branch has not been deployed

No deployments
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.

1 participant