Update existing SNV collection with additional SNVs by peterpru · Pull Request #177 · Clinical-Genomics/loqusdb

peterpru · 2026-09-08T06:41:56Z

Description

This PR adds the option to update an existing set of SNVs with additional SNVs, without modifying the already uploaded variants. This is a solution for uploading MT variants of cases that have already been run, but did not have their MT variants uploaded (https://github.com/Clinical-Genomics/MTP-RAREDISEASE/issues/172). We do not want to reupload all these cases, only to add to them.

This should run as:

loqusdb update \
  --variant-file additional.vcf \
  --family-file case.ped \
  --add-to-existing-snv

Added

Added loqusdb update --add-to-existing-snv to append SNVs to an existing case without replacing its original SNV upload.
Added --ignore-gq-if-unset support to loqusdb update.

Changed

loqusdb update now accepts the global --genome-build and --keep-chr-prefix options.

How to prepare for test

ssh to hasta.scilifelab.se
Use stage: us
Paxa the environment: paxa
Install on stage (example for hasta):
bash /home/proj/production/servers/resources/hasta.scilifelab.se/update-tool-stage.sh -e S_loqusdb -t loqusdb -b update-existing-snv-variants

How to test

Do regular upload of NIST sample:
/home/proj/stage/bin/miniconda3/envs/S_loqusdb/bin/loqusdb --config /home/proj/stage/servers/config/hasta.scilifelab.se/loqusdb-wgs-stage.yaml --keep-chr-prefix --genome-build GRCh38 load --case-id evolvedsalmon --variant-file /home/proj/production/housekeeper-bundles/evolvedsalmon/2026-05-13/evolvedsalmon_snv_ranked_clinical.vcf.gz --check-profile /home/proj/production/housekeeper-bundles/evolvedsalmon/2026-05-13/evolvedsalmon_snv_ranked_clinical.vcf.gz --family-file /home/proj/production/housekeeper-bundles/evolvedsalmon/2026-05-13/evolvedsalmon.ped --gq-threshold 10 --hard-threshold 0.95 --soft-threshold 0.9
Do the addition with the new flag --add-to-existing-snv:
/home/proj/stage/bin/miniconda3/envs/S_loqusdb/bin/loqusdb --config /home/proj/stage/servers/config/hasta.scilifelab.se/loqusdb-wgs-stage.yaml --keep-chr-prefix --genome-build GRCh38 update --case-id evolvedsalmon --variant-file /home/proj/production/housekeeper-bundles/evolvedsalmon/2026-05-13/evolvedsalmon_mt_ranked_clinical.vcf.gz --family-file /home/proj/production/housekeeper-bundles/evolvedsalmon/2026-05-13/evolvedsalmon.ped --ignore-gq-if-unset --add-to-existing-snv

Expected test outcome

Check that the case uploads

The addition adds the new MT variants when using loqusdb update with --add-to-existing-snv:

Checking in MongoDB compass shows the number of variants in the DB to be 608989, which is the 608974 of the SNV upload + the 15 of the MT upload.

Query mongoDB to show MT variants added:

{
  _id: "$chrom",
  count: {
    $sum: 1
  }
}

Bonus test, the case variants should have been updated to the new number:
After loqusDB load it shows the nr of variants in the VCF:

After loqusdb update it shows the number of variants in the VCF + MT VCF:

Review

Tests executed by
"Merge and deploy" approved by
Thanks for filling in who performed the code review and the test!

This version is a

MAJOR - when you make incompatible API changes
MINOR - when you add functionality in a backwards compatible manner
PATCH - when you make backwards compatible bug fixes or documentation/instructions

Implementation Plan

Document in ...
Deploy this branch
Inform to ...

coveralls · 2026-09-08T06:44:04Z

Coverage Report for CI Build 34480272501

Warning

No base build found for commit 3db9c19 on main.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 72.556%

Details

Patch coverage: 4 uncovered changes across 2 files (16 of 20 lines covered, 80.0%).

Uncovered Changes

File	Changed	Covered	%
loqusdb/commands/update.py	7	5	71.43%
loqusdb/utils/update.py	11	9	81.82%
Total (3 files)	20	16	80.0%

Coverage Regressions

Requires a base build to compare against. How to fix this →

Coverage Stats


Relevant Lines:	1698
Covered Lines:	1232
Line Coverage:	72.56%
Coverage Strength:	1.45 hits per line

💛 - Coveralls

northwestwitch

Sorry, but I am not super fond of this solution. As I wrote in a code comment, it's adding complexity to the general case loader used when the case is first loaded.

I'm sure it works fine and does the loading of the variants, but if I got the code right, it doesn't modify the original case document, which will still contain the vcf file with only the autosomes. Something like this:

So if you want to remove and re-upload the variants for the case in the future the MT ones won't be loaded.

Solutions I like

Merging the files - autosomes + MT under the automsomes file and reupload all cases
Updating the case document with a new type of VCF file, like mt_vcf_path? via the loqusdb update command. And then re-upload all variants, including 3 categories this time (sns, mt_snvs, svs). I think this would be the cleanest way to go, after solution 1..

What do you think @dnil?

peterpru · 2026-09-09T07:11:35Z

Sorry, but I am not super fond of this solution. As I wrote in a code comment, it's adding complexity to the general case loader used when the case is first loaded.

I'm sure it works fine and does the loading of the variants, but if I got the code right, it doesn't modify the original case document, which will still contain the vcf file with only the autosomes. Something like this:
So if you want to remove and re-upload the variants for the case in the future the MT ones won't be loaded.
Solutions I like

Merging the files - autosomes + MT under the automsomes file and reupload all cases

Updating the case document with a new type of VCF file, like mt_vcf_path? via the loqusdb update command. And then re-upload all variants, including 3 categories this time (sns, mt_snvs, svs). I think this would be the cleanest way to go, after solution 1..

What do you think @dnil?

I changed it so the command is now under loqusdb update, I agree that makes more sense. I had to add some of the options that were only in loqusdb load, and I will now test it in the morning, and update the output in the test.

I also added that it updates the nr_variants, I'll test it now.

I like the second option you mention, maybe something like additional_vcf_path? That can be null for files that do not have anything extra uploaded (just like when no SVs are uploaded). That way we would not have to reupload anything, and could just trigger the update command for those cases.

northwestwitch · 2026-09-09T07:22:34Z

I like the second option you mention, maybe something like additional_vcf_path? That can be null for files that do not have anything extra uploaded (just like when no SVs are uploaded). That way we would not have to reupload anything, and could just trigger the update command for those cases.

Nice, let's try like this. We also have to keep in mind that invasive changes might break loqusdbapi, which uses several loqusdb functions in background. Let's check before merging

peterpru · 2026-09-09T07:55:53Z

I like the second option you mention, maybe something like additional_vcf_path? That can be null for files that do not have anything extra uploaded (just like when no SVs are uploaded). That way we would not have to reupload anything, and could just trigger the update command for those cases.

Nice, let's try like this. We also have to keep in mind that invasive changes might break loqusdbapi, which uses several loqusdb functions in background. Let's check before merging

Now it looks like this, with additional VCF paths as an array, so it should allow us to add multiple files if needed:

northwestwitch · 2026-09-09T08:05:46Z

I like the second option you mention, maybe something like additional_vcf_path? That can be null for files that do not have anything extra uploaded (just like when no SVs are uploaded). That way we would not have to reupload anything, and could just trigger the update command for those cases.

Nice, let's try like this. We also have to keep in mind that invasive changes might break loqusdbapi, which uses several loqusdb functions in background. Let's check before merging

Now it looks like this, with additional VCF paths as an array, so it should allow us to add multiple files if needed:

If you make it an array of dictionaries you can also assign the category of the variants stored in the vcf file, something like:

additional_vcf_paths: [
{path:"whavetever", category: "snv"},
]

So you don't have to hardcode the category in the code later on. And opens up to have additional SVs as well in the future

Just an idea!

peterpru · 2026-09-09T12:33:48Z

If you make it an array of dictionaries you can also assign the category of the variants stored in the vcf file, something like:

additional_vcf_paths: [ {path:"whavetever", category: "snv"}, ]

So you don't have to hardcode the category in the code later on. And opens up to have additional SVs as well in the future

Just an idea!

Nice, added it now.

northwestwitch

I think it's definitely getting there but there are some things I would change. Mainly:

If you allow extra VCF files to be both SNVs and SVs, I think you should allow both to be loaded, and not hard code that the additional file should be SNVs
I'm not sure about the variants existence check before the insert: it could slow down the whole thing, which we don't want

Nice with the check of build and chromosome profix. It was completely missing and potentially a bug! 💯

This could use a review/suggestion also from @dnil that perhaps has a better insight than me on the loqus database?

northwestwitch · 2026-09-10T06:43:12Z

    help="Specify the maximum window size for svs",
 )
+@click.option(
+    "--add-to-existing-snv",


Why is it named like this if the additional file can be a SV VCF?

northwestwitch · 2026-09-10T06:48:53Z

    help="Specify the maximum window size for svs",
 )
+@click.option(
+    "--add-to-existing-snv",


I would make this general. Like if you pass --sv-variants then it adds a new SV file in additional_vcf_paths. How about renaming it like is_additional or something?

Because you created a flexible structure in the new field anyway:

northwestwitch · 2026-09-10T06:49:12Z

+    is_flag=True,
+    default=False,
+    show_default=True,
+    help="Add SNVs to an existing case without replacing its stored SNV VCF",


Suggested change

help="Add SNVs to an existing case without replacing its stored SNV VCF",

help="Associate an additional VCF with a case without replacing existing VCFs or variants",

northwestwitch · 2026-09-10T06:55:41Z

        ctx.abort()

+    if add_to_existing_snv and (not variant_file or sv_variants):
+        LOG.warning("--add-to-existing-snv requires --variant-file and no --sv-variants")


I would not make this check. Instead I would check that you provide either a SNV or a SV file.

northwestwitch · 2026-09-10T07:02:50Z

        variant_sv_path = os.path.abspath(sv_variants)

    adapter = ctx.obj["adapter"]
+    genome_build = ctx.obj["genome_build"]


northwestwitch · 2026-09-10T07:09:13Z

        raise CaseError("Case {} does not exist in database".format(case_obj["case_id"]))

+    if add_to_existing_snv:
+        if not existing_case.get("vcf_path"):


Why? I don't think it matters for the sake of adding extra VCFs and variants, or perhaps I'm not considering something?

northwestwitch · 2026-09-10T07:20:30Z

+                for variant in variants
+                if variant
+                and case_obj["case_id"]
+                not in (adapter.get_variant(variant) or {}).get("families", [])


I see where this comes from. I don't know, this lookup for each variant has the potential of slowing down the whole process, as the loqus database is huge.. I guess we could entirely skip this code if we assume that we are not going to insert variants that already exist for the case, but mmm

northwestwitch · 2026-09-10T09:50:17Z

Another thing that came to my mind: in the case document, each sample has an assigned index (column in the VCF file).

So if you add the additional SNV file, than you have to be sure that the individuals in the file have the order present in the case document,. See the fields in the case document for a demo case:

Are we sure the sample order is respected in the new files? Otherwise the sample stats will be scrambled

northwestwitch · 2026-09-10T09:56:06Z

I don't know, The more I think about this and the more complicated it seems..

peterpru · 2026-09-10T12:35:29Z

I don't know, The more I think about this and the more complicated it seems..

Thank you for the review. I can see your point, this is getting more messy the further we get. The sample order is also not something I can promise is the same every time. This PR came from a need to have the MT variants added for each case. I will check how involved it would be to manually create the concatenated files for these cases, and re-upload each case.

Edit, assuming only 15 minutes to delete and load each case, we are looking at 21 days' worth of uploading, that does not include concatenating and storing the files.
Edit2, sample order seems to be preserved after checking older cases.

peterpru · 2026-09-14T14:33:35Z

Currently uploaded mt variants for 1589 singleton cases (all eligible WGS hg38 cases since the start of RD hg38 until this week). This leaves out 253 duo and trio cases (mix of WGS and WES; WES don't have the MT file) that we can add in the future or ignore, depending on whether we trust the sample matching for multi-sample cases.

Using the .sh script below:

#!/bin/bash

input_file="RD_cases_singleton.csv"

while IFS= read -r case; do
    echo "now running case $case"
    /home/proj/stage/bin/miniconda3/envs/S_loqusdb/bin/loqusdb \
        --config /home/proj/production/servers/config/hasta.scilifelab.se/loqusdb-wgs.yaml \
        --keep-chr-prefix \
        --genome-build GRCh38 update \
        --case-id "$case" \
        --variant-file /home/proj/production/housekeeper-bundles/"$case"/*/"${case}_mt_ranked_clinical.vcf.gz" \
        --family-file /home/proj/production/housekeeper-bundles/"$case"/*/"${case}.ped" \
        --ignore-gq-if-unset \
        --add-to-existing-snv
done < "$input_file"

peterpru · 2026-09-15T02:30:18Z

Closed, as the needed files are now part of the database.

northwestwitch · 2026-09-15T05:06:06Z

Great job @peterpru! 👏🏻 👏🏻 🥳 🥇

update existing snv variants

96df074

peterpru self-assigned this Sep 8, 2026

peterpru added 2 commits September 8, 2026 08:46

run black and changelog

d5461a7

black using the latest version

27e4249

peterpru marked this pull request as ready for review September 8, 2026 07:49

peterpru requested review from a team as code owners September 8, 2026 07:49

northwestwitch reviewed Sep 8, 2026

View reviewed changes

Comment thread loqusdb/commands/load.py Outdated

Comment thread loqusdb/utils/load.py Outdated

peterpru added 4 commits September 9, 2026 07:35

move to update and increment nr_variants

966ddb7

add genome build to update

095ead5

fix tests

073d4e6

add ignore-gq-if-unset to update

dc52cb1

fix test

75b19cb

northwestwitch requested changes Sep 10, 2026

View reviewed changes

peterpru force-pushed the update-existing-snv-variants branch from 203df44 to 75b19cb Compare September 10, 2026 13:03

peterpru closed this Sep 15, 2026

	help="Add SNVs to an existing case without replacing its stored SNV VCF",
	help="Associate an additional VCF with a case without replacing existing VCFs or variants",

Conversation

peterpru commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Added

Changed

How to prepare for test

How to test

Expected test outcome

Review

This version is a

Implementation Plan

Uh oh!

coveralls commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for CI Build 34480272501

Coverage: 72.556%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

northwestwitch left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

peterpru commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

northwestwitch commented Sep 9, 2026

Uh oh!

peterpru commented Sep 9, 2026

Uh oh!

northwestwitch commented Sep 9, 2026

Uh oh!

peterpru commented Sep 9, 2026

Uh oh!

northwestwitch left a comment

Choose a reason for hiding this comment

Uh oh!

northwestwitch Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

northwestwitch Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

northwestwitch Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

northwestwitch Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

northwestwitch Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

northwestwitch Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

northwestwitch Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

northwestwitch commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

northwestwitch commented Sep 10, 2026

Uh oh!

peterpru commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peterpru commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peterpru commented Sep 15, 2026

Uh oh!

northwestwitch commented Sep 15, 2026

Uh oh!

Reviewers

peterpru commented Sep 8, 2026 •

edited

Loading

coveralls commented Sep 8, 2026 •

edited

Loading

northwestwitch left a comment •

edited

Loading

peterpru commented Sep 9, 2026 •

edited

Loading

northwestwitch commented Sep 10, 2026 •

edited

Loading

peterpru commented Sep 10, 2026 •

edited

Loading

peterpru commented Sep 14, 2026 •

edited

Loading