Dev

.github/workflows/vlm-unit-tests.yaml

@@ -21,7 +21,7 @@ on:
 jobs:
   vlm_clickhouse:
     runs-on: ubuntu-latest
-    container: hailgenetics/hail:0.2.128
+    container: python:3.11-slim-bullseye
 
     services:
       clickhouse:
@@ -44,6 +44,7 @@ jobs:
       - name: Install dependencies
         run: |
           python3 -m pip install --upgrade pip wheel
+          pip install -r vlm/requirements.txt
           pip install -r vlm/requirements-test.txt
       - name: Set up Clickhouse Settings and Data
         run: python3 vlm/setup_clickhouse_test_data.py clickhouse 8123 clickhouse_test_user clickhouse_test_password
@@ -57,5 +58,5 @@ jobs:
           export CLICKHOUSE_VLM_PASSWORD=vlm_test_password
           export CLICKHOUSE_DATABASE=test_seqr
           coverage run --source="./vlm" --omit="./vlm/__main__.py","./vlm/setup_clickhouse_test_data.py" -m pytest vlm/
-          coverage report --fail-under=95
+          coverage report -m --fail-under=95
 

clickhouse_search/backend/table_models.py

@@ -81,13 +81,13 @@ def base_fields(cls):
         return [(field.db_column or field.name, field) for field in cls._meta.local_fields if field.name != 'key']
 
     @classmethod
-    def dict_get_expression(cls, *expressions, field_names=None, force_tuple=False, **kwargs):
+    def dict_get_expression(cls, *expressions, key_template='%(expressions)s', field_names=None, force_tuple=False, **kwargs):
         base_fields = cls.base_fields()
         if field_names:
             base_fields = [f for f in base_fields if f[0] in field_names]
         output_field = base_fields[0][1] if len(base_fields) == 1  and not force_tuple else NamedTupleField(base_fields)
         dict_get_func = Func(*expressions, output_field=output_field)
-        dict_get_func.template = cls.dict_get_sql('%(expressions)s', [field_name for field_name, _  in base_fields], **kwargs)
+        dict_get_func.template = cls.dict_get_sql(key_template, [field_name for field_name, _  in base_fields], **kwargs)
         if force_tuple and len(base_fields) == 1:
             dict_get_func.template = f'tuple({dict_get_func.template})'
         return dict_get_func

clickhouse_search/fixtures/clickhouse_discovery_variants.json

@@ -0,0 +1,88 @@
+[{
+    "model": "clickhouse_search.entriessnvindel",
+    "pk": 100,
+    "fields": {
+      "key": 100,
+       "project_guid": "R0001_1kg",
+      "family_guid": "F000002_2",
+      "sample_type": "WGS",
+      "xpos": 1248367227,
+      "is_gnomad_gt_5_percent": false,
+      "is_annotated_in_any_gene": true,
+      "geneId_ids": [6, 48],
+      "filters": [],
+      "sign": 1,
+      "calls": [
+        ["HG00733", 1, 99, 0.6530612111091614, 49],
+        ["HG00731", 0, 48, 1, 16]
+      ]
+    }
+}, {
+    "model": "clickhouse_search.keylookupsnvindel",
+    "pk": 100,
+    "fields": {
+      "key": 100,
+      "variant_id": "1-248367227-TC-T"
+    }
+}, {
+    "model": "clickhouse_search.variantssnvindel",
+    "pk": 100,
+    "fields": {
+      "key": 100,
+      "sorted_motif_feature_consequences": [],
+      "sorted_regulatory_feature_consequences": [],
+      "sorted_transcript_consequences": []
+    }
+},{
+    "model": "clickhouse_search.variantdetailssnvindel",
+    "pk": 100,
+    "fields": {
+      "key": 100,
+      "variant_id": "1-248367227-TC-T",
+      "rsid": null,
+      "caid": "CA1501729",
+      "lifted_over_chrom": "1",
+      "lifted_over_pos": null,
+      "transcripts": [],
+      "sorted_motif_feature_consequences": [],
+      "sorted_regulatory_feature_consequences": []
+    }
+}, {
+    "model": "clickhouse_search.entriessnvindel",
+    "pk": 100,
+    "fields": {
+      "key": 100,
+      "project_guid": "R0004_non_analyst_project",
+      "family_guid": "F000014_14",
+      "sample_type": "WGS",
+      "xpos": 1248367227,
+      "is_gnomad_gt_5_percent": false,
+      "is_annotated_in_any_gene": false,
+      "geneId_ids": [],
+      "filters": [],
+      "sign": 1,
+      "calls": [
+        ["NA21234", 2, 99, 0, 49]
+      ]
+    }
+}, {
+    "model": "clickhouse_search.entriessnvindel",
+    "pk": 100,
+    "fields": {
+      "key": 100,
+      "project_guid": "R0003_test",
+      "family_guid": "F000012_12",
+      "sample_type": "WGS",
+      "xpos": 1248367227,
+      "is_gnomad_gt_5_percent": false,
+      "is_annotated_in_any_gene": false,
+      "geneId_ids": [],
+      "filters": [],
+      "sign": 1,
+      "calls": [
+        ["NA20889", 1, 99, 0.0, 71],
+        ["NA20888", 1, 99, 0.5555556, 9]
+      ]
+    }
+}
+]

clickhouse_search/fixtures/clickhouse_saved_variants.json

@@ -172,45 +172,6 @@
       "sift": null,
       "vest": null
     }
-}, {
-    "model": "clickhouse_search.keylookupsnvindel",
-    "pk": 100,
-    "fields": {
-      "key": 100,
-      "variant_id": "1-248367227-TC-T"
-    }
-}, {
-    "model": "clickhouse_search.variantdetailssnvindel",
-    "pk": 100,
-    "fields": {
-      "key": 100,
-      "variant_id": "1-248367227-TC-T",
-      "rsid": null,
-      "caid": "CA1501729",
-      "lifted_over_chrom": "1",
-      "lifted_over_pos": null,
-      "transcripts": [],
-      "sorted_motif_feature_consequences": [],
-      "sorted_regulatory_feature_consequences": []
-    }
-}, {
-    "model": "clickhouse_search.entriessnvindel",
-    "pk": 100,
-    "fields": {
-      "key": 100,
-      "project_guid": "R0004_non_analyst_project",
-      "family_guid": "F000014_14",
-      "sample_type": "WGS",
-      "xpos": 1248367227,
-      "is_gnomad_gt_5_percent": false,
-      "is_annotated_in_any_gene": false,
-      "geneId_ids": [],
-      "filters": [],
-      "sign": 1,
-      "calls": [
-        ["NA21234", 2, 99, 0, 49]
-      ]
-    }
 }, {
     "model": "clickhouse_search.keylookupsv",
     "pk": 101,

clickhouse_search/management/commands/register_caids.py

@@ -108,8 +108,8 @@ class AlleleRegistryError:
     def from_api_response(cls, response: dict):
         return cls(
             error_type=response["errorType"],
-            description=response["description"],
-            message=response["message"],
+            description=response.get("description"),
+            message=response.get("message"),
             input_line=response.get("inputLine"),
         )
 
@@ -154,8 +154,10 @@ def handle_api_response(
     genome_version: Literal[GENOME_VERSION_GRCh37, GENOME_VERSION_GRCh38],
     response: requests.Response,
 ) -> dict[str, str]:
+    if not response.ok:
+        raise HTTPError(f"Unexpected AR response code: {response.status_code}")
     response_json = response.json()
-    if not response.ok or "errorType" in response_json:
+    if "errorType" in response_json:
         error = AlleleRegistryError.from_api_response(response_json)
         logger.error(error)
         raise HTTPError(error.message)

clickhouse_search/management/commands/reload_clinvar_all_variants.py

@@ -20,27 +20,19 @@
 
 logger = logging.getLogger(__name__)
 
-def replace_underscores_with_spaces(value: Union[str, list[str]]) -> Union[str, list[str]]:
-    if isinstance(value, str):
-        return value.replace('_', ' ')
-    elif isinstance(value, list):
-        return [s.replace('_', ' ') for s in value]
-    raise TypeError("Expected str or list[str]")
+def replace_underscores_with_spaces(value: list[str]) -> list[str]:
+    return [s.replace('_', ' ') for s in value]
 
-def replace_spaces_with_underscores(value: Union[str, list[str], list[tuple[str, int]]]) -> Union[str, list[str]]:
-    if isinstance(value, str):
-        return value.replace(' ', '_')
-    elif isinstance(value, list):
-        if len(value) > 0 and isinstance(value[0], tuple):
-            return [(t[0].replace(' ', '_'), t[1]) for t in value]
-        return [s.replace(' ', '_') for s in value]
-    raise TypeError("Expected str or list[str]")
+def replace_spaces_with_underscores(value: Union[list[str], list[tuple[str, int]]]) -> list[str]:
+    if len(value) > 0 and isinstance(value[0], tuple):
+        return [(t[0].replace(' ', '_'), t[1]) for t in value]
+    return [s.replace(' ', '_') for s in value]
 
 BATCH_SIZE = 1000
 CLINVAR_ASSERTIONS = replace_underscores_with_spaces(ClinvarAllVariantsSnvIndel.CLINVAR_ASSERTIONS)
-CLINVAR_CONFLICTING_CLASSICATIONS_OF_PATHOGENICITY = replace_underscores_with_spaces(ClinvarAllVariantsSnvIndel.CLINVAR_CONFLICTING_CLASSICATIONS_OF_PATHOGENICITY)
+CLINVAR_CONFLICTING_CLASSICATIONS_OF_PATHOGENICITY = replace_underscores_with_spaces([ClinvarAllVariantsSnvIndel.CLINVAR_CONFLICTING_CLASSICATIONS_OF_PATHOGENICITY])[0]
 CLINVAR_CONFLICTING_DATA_FROM_SUBMITTERS = 'conflicting data from submitters'
-CLINVAR_DEFAULT_PATHOGENICITY = replace_underscores_with_spaces(ClinvarAllVariantsSnvIndel.CLINVAR_DEFAULT_PATHOGENICITY)
+CLINVAR_DEFAULT_PATHOGENICITY = replace_underscores_with_spaces([ClinvarAllVariantsSnvIndel.CLINVAR_DEFAULT_PATHOGENICITY])[0]
 CLINVAR_PATHOGENICITIES = replace_underscores_with_spaces(ClinvarAllVariantsSnvIndel.CLINVAR_PATHOGENICITIES)
 CLINVAR_GOLD_STARS_LOOKUP = {
     'no classification for the single variant': 0,
@@ -228,7 +220,7 @@ def extract_variant_info(elem: xml.etree.ElementTree.Element, new_version: str,
     props = {
         'version': new_version,
         'allele_id': allele_id,
-        'pathogenicity': replace_spaces_with_underscores(pathogenicity),
+        'pathogenicity': replace_spaces_with_underscores([pathogenicity])[0],
         'assertions': replace_spaces_with_underscores(assertions),
         'conflicting_pathogenicities': replace_spaces_with_underscores(conflicting_pathogenicities),
         'gold_stars': gold_stars,
@@ -261,7 +253,7 @@ def parse_clinvar_file(gzipped_file, existing_version_obj, model_to_batch, unenu
             if existing_version_obj:
                 if existing_version_obj.version == new_version:
                     logger.info(f'Clinvar ClickHouse tables already successfully updated to {new_version}, gracefully exiting.')
-                    return new_version
+                    return None
             logger.info( f'Updating Clinvar ClickHouse tables to {new_version} from {existing_version_obj and existing_version_obj.version}.')
             # Drop any currently existing variants in the table that may exist due to a
             # previously failed partial run.  Note that we validate that the Postgresql existing version
@@ -306,6 +298,9 @@ def handle(self, *args, **options):
             with gzip.open(tmpfile.name, 'rb') as gzipped_file:
                 new_version = parse_clinvar_file(gzipped_file, existing_version_obj, model_to_batch, unenumerated_value_alerts)
 
+        if not new_version:
+            return
+
         for model, batch in model_to_batch.items():
             if batch:
                 model.objects.using('clickhouse_write').bulk_create(batch)

clickhouse_search/management/commands/set_saved_variant_key.py

@@ -19,7 +19,6 @@ class Command(BaseCommand):
     def handle(self, *args, **options):
         variant_ids = SavedVariant.objects.filter(
             key__isnull=True, family__project__genome_version=GENOME_VERSION_GRCh38,
-            saved_variant_json__populations__isnull=False, # Omit manual variants
         ).values_list('variant_id', flat=True).distinct()
         ids_by_dataset_type = {
             Dataset.DATASET_TYPE_VARIANT_CALLS: [], Dataset.DATASET_TYPE_MITO_CALLS: [], Dataset.DATASET_TYPE_SV_CALLS: [],
@@ -59,11 +58,6 @@ def handle(self, *args, **options):
         if no_keys_37:
             self._resolve_missing_variants(no_keys_37, GENOME_VERSION_GRCh37)
 
-        num_updated = SavedVariant.objects.filter(key__isnull=False).exclude(saved_variant_json={}).update(
-            saved_variant_json={},
-        )
-        logger.info(f'Cleared saved json for {num_updated} variants with keys')
-
         logger.info('Done')
 
     @staticmethod
@@ -98,32 +92,32 @@ def _set_variant_keys(variants_ids, dataset_type, genome_version=GENOME_VERSION_
         return no_key
 
     @classmethod
-    def _query_missing_variants(cls, variant_ids, variant_fields, genome_version=GENOME_VERSION_GRCh38):
+    def _query_missing_variants(cls, variant_ids, variant_fields=None, genome_version=GENOME_VERSION_GRCh38):
         missing_variants = SavedVariant.objects.filter(
             variant_id__in=variant_ids, family__project__genome_version=genome_version,
         )
         num_missing = missing_variants.count()
         missing_with_data_qs = missing_variants.filter(family__individual__active_datasets__isnull=False).distinct()
         missing_with_search_data = missing_with_data_qs.values(
-            'variant_id', *variant_fields,
+            'variant_id', *(variant_fields or []),
         ).annotate(family_ids=ArrayAgg('family__family_id', distinct=True)).order_by('variant_id')
         return missing_with_search_data, num_missing
 
     @classmethod
     def _resolve_missing_variants(cls, variant_ids, genome_version):
         missing_with_search_data, num_missing = cls._query_missing_variants(
-            variant_ids, ['saved_variant_json__populations__seqr__ac'], genome_version,
+            variant_ids, genome_version=genome_version,
         )
         num_data= len(missing_with_search_data)
-        in_backend = [
-            f"{var['variant_id']} - {'; '.join(var['family_ids'])}"
-            for var in missing_with_search_data if var['saved_variant_json__populations__seqr__ac']
-        ]
+
         logger.info(
-            f'{num_missing} variants have no key, {num_missing - num_data} of which have no search data, {num_data - len(in_backend)} of which are absent from the hail backend.'
+            f'{num_missing} variants have no key, {num_missing - num_data} of which have no search data.'
         )
-        if in_backend:
-            logger.info(f'{len(in_backend)} remaining variants: {", ".join(in_backend)}')
+        if missing_with_search_data:
+            summary = [
+                f"{var['variant_id']} - {'; '.join(var['family_ids'])}" for var in missing_with_search_data
+            ]
+            logger.info(f'{len(missing_with_search_data)} remaining variants: {", ".join(summary)}')
 
     @classmethod
     def _resolve_reloaded_svs(cls, variant_ids):

clickhouse_search/management/tests/register_caids_tests.py

@@ -50,6 +50,26 @@
             },
         ],
     },
+    {
+        '@id': 'http://reg.genome.network/allele/CA16716504',
+        'genomicAlleles': [
+            {
+                'chromosome': '1',
+                'coordinates': [
+                    {
+                        'allele': 'C',
+                        'end': 10131,
+                        'referenceAllele': '',  # ref allele is ''
+                        'start': 10131,
+                    },
+                ],
+                'referenceGenome': 'GRCh38',
+            },
+        ],
+        'externalRecords': {
+            'gnomAD_4': [{'id': '1-91511686-91511734'}], # has invalid gnomad ID
+        },
+    },
     {
         '@id': 'http://reg.genome.network/allele/CA997563845',
         'genomicAlleles': [
@@ -135,6 +155,21 @@ def test_bad_responses(self, mock_safe_post_to_slack, mock_logger):
             call_command("register_caids", batch_size=5)
         mock_safe_post_to_slack.assert_not_called()
 
+        responses.add(
+            responses.PUT,
+            "https://reg.genome.network/alleles",
+            match=[
+                responses.matchers.query_param_matcher({
+                    "file": "vcf",
+                    "fields": "none @id genomicAlleles externalRecords.gnomAD_4.id",
+                }, strict_match=False),
+            ],
+            status=400,
+        )
+        with self.assertRaisesMessage(CommandError, 'Failed in 38/ClingenAlleleRegistry curr_key: 3'):
+            call_command("register_caids", batch_size=5)
+        mock_safe_post_to_slack.assert_not_called()
+
         responses.reset()
         mock_safe_post_to_slack.reset_mock()
         mock_logger.reset_mock()
@@ -224,7 +259,7 @@ def test_register_caids(self, mock_safe_post_to_slack, mock_logger):
         dv_38 = DataVersions.objects.get(data_model_name='38/ClingenAlleleRegistry')
         self.assertEqual(dv_38.version, '10')
         mock_logger.info.assert_called_with(
-            "2 registered variant(s) cannot be mapped back to ours. \n"
+            "3 registered variant(s) cannot be mapped back to ours. \n"
             "First unmappable variant:\n{'@id': 'http://reg.genome.network/allele/CA16716503', 'genomicAlleles': [{'chromosome': '1', 'coordinates': [{'allele': 'C', 'end': 10131, 'referenceAllele': '', 'start': 10131}], 'referenceGenome': 'GRCh38'}]}"
         )
         mock_logger.warning.assert_called_with(

clickhouse_search/management/tests/reload_clinvar_all_variants_tests.py

@@ -103,6 +103,8 @@ def test_new_version_already_exists(self, mock_logger, mock_safe_post_to_slack):
         responses.add(responses.GET, WEEKLY_XML_RELEASE, status=200, body=gzip.compress(WEEKLY_XML_RELEASE_DATA.encode()), stream=True)
         call_command('reload_clinvar_all_variants')
         mock_logger.assert_called_with('Clinvar ClickHouse tables already successfully updated to 2025-06-30, gracefully exiting.')
+        mock_safe_post_to_slack.assert_not_called()
+        self.assertEqual(DataVersions.objects.get(data_model_name='Clinvar').version, '2025-06-30')
 
     @responses.activate
     def test_parse_variants_all_types(self, mock_logger, mock_safe_post_to_slack):