Merge pull request #720 from broadinstitute/dev

Dev

Author: bpblanken

v03_pipeline/lib/misc/io.py

@@ -146,7 +146,12 @@ def select_relevant_fields(
     dataset_type: DatasetType,
 ) -> hl.MatrixTable:
     mt = mt.select_globals()
-    mt = mt.select_rows(*dataset_type.row_fields)
+    optional_row_fields = [
+        row_field
+        for row_field in dataset_type.optional_row_fields
+        if hasattr(mt, row_field)
+    ]
+    mt = mt.select_rows(*dataset_type.row_fields, *optional_row_fields)
     mt = mt.select_cols(*dataset_type.col_fields)
     return mt.select_entries(*dataset_type.entries_fields)
 

v03_pipeline/lib/model/dataset_type.py

@@ -5,7 +5,6 @@
 
 from v03_pipeline.lib.annotations import gcnv, mito, shared, snv_indel, sv
 from v03_pipeline.lib.model.definitions import ReferenceGenome
-from v03_pipeline.lib.model.environment import Env
 
 MITO_MIN_HOM_THRESHOLD = 0.95
 ZERO = 0.0
@@ -65,19 +64,23 @@ def entries_fields(
             ],
         }[self]
 
+    @property
+    def optional_row_fields(
+        self,
+    ) -> list[str]:
+        return {
+            DatasetType.SNV_INDEL: ['info'],
+            DatasetType.MITO: [],
+            DatasetType.SV: [],
+            DatasetType.GCNV: [],
+        }[self]
+
     @property
     def row_fields(
         self,
     ) -> list[str]:
         return {
-            DatasetType.SNV_INDEL: (
-                # The "info" field is used by one of the relatedness
-                # check methods and not by an annotation method.  We
-                # only want to keep the field for callsets that require it.
-                ['rsid', 'filters', 'info']
-                if Env.CHECK_SEX_AND_RELATEDNESS
-                else ['rsid', 'filters']
-            ),
+            DatasetType.SNV_INDEL: ['rsid', 'filters'],
             DatasetType.MITO: [
                 'rsid',
                 'filters',

v03_pipeline/lib/model/environment.py

@@ -5,7 +5,6 @@
 ACCESS_PRIVATE_REFERENCE_DATASETS = (
     os.environ.get('ACCESS_PRIVATE_REFERENCE_DATASETS') == '1'
 )
-CHECK_SEX_AND_RELATEDNESS = os.environ.get('CHECK_SEX_AND_RELATEDNESS') == '1'
 REFERENCE_DATA_AUTO_UPDATE = os.environ.get('REFERENCE_DATA_AUTO_UPDATE') == '1'
 HAIL_TMPDIR = os.environ.get('HAIL_TMPDIR', '/tmp')  # noqa: S108
 HAIL_SEARCH_DATA = os.environ.get('HAIL_SEARCH_DATA', '/hail-search-data')
@@ -25,7 +24,6 @@
 @dataclass
 class Env:
     ACCESS_PRIVATE_REFERENCE_DATASETS: bool = ACCESS_PRIVATE_REFERENCE_DATASETS
-    CHECK_SEX_AND_RELATEDNESS: bool = CHECK_SEX_AND_RELATEDNESS
     REFERENCE_DATA_AUTO_UPDATE: bool = REFERENCE_DATA_AUTO_UPDATE
     HAIL_TMPDIR: str = HAIL_TMPDIR
     HAIL_SEARCH_DATA: str = HAIL_SEARCH_DATA

v03_pipeline/lib/tasks/write_metadata_for_run.py

@@ -29,6 +29,10 @@ class WriteMetadataForRunTask(BaseHailTableTask):
         default=True,
         parsing=luigi.BoolParameter.EXPLICIT_PARSING,
     )
+    check_sex_and_relatedness = luigi.BoolParameter(
+        default=True,
+        parsing=luigi.BoolParameter.EXPLICIT_PARSING,
+    )
     run_id = luigi.Parameter()
 
     def output(self) -> luigi.Target:
@@ -56,6 +60,7 @@ def requires(self) -> luigi.Task:
                 self.ignore_missing_samples_when_subsetting,
                 self.ignore_missing_samples_when_remapping,
                 self.validate,
+                self.check_sex_and_relatedness,
             )
             for (
                 callset_path,

v03_pipeline/lib/tasks/write_metadata_for_run_test.py

@@ -24,6 +24,7 @@ def test_write_metadata_for_run_task(self) -> None:
             project_remap_paths=[TEST_REMAP_2, TEST_REMAP_2],
             project_pedigree_paths=[TEST_PEDIGREE_3, TEST_PEDIGREE_4],
             validate=False,
+            check_sex_and_relatedness=False,
             run_id='run_123456',
         )
         worker.add(write_metadata_for_run_task)

v03_pipeline/lib/tasks/write_remapped_and_subsetted_callset.py

@@ -9,7 +9,6 @@
 from v03_pipeline.lib.misc.io import does_file_exist, import_pedigree, import_remap
 from v03_pipeline.lib.misc.pedigree import parse_pedigree_ht_to_families
 from v03_pipeline.lib.misc.sample_ids import remap_sample_ids, subset_samples
-from v03_pipeline.lib.model import Env
 from v03_pipeline.lib.paths import remapped_and_subsetted_callset_path
 from v03_pipeline.lib.tasks.base.base_write_task import BaseWriteTask
 from v03_pipeline.lib.tasks.files import GCSorLocalTarget, RawFileTask
@@ -37,6 +36,10 @@ class WriteRemappedAndSubsettedCallsetTask(BaseWriteTask):
         default=True,
         parsing=luigi.BoolParameter.EXPLICIT_PARSING,
     )
+    check_sex_and_relatedness = luigi.BoolParameter(
+        default=False,
+        parsing=luigi.BoolParameter.EXPLICIT_PARSING,
+    )
 
     def output(self) -> luigi.Target:
         return GCSorLocalTarget(
@@ -64,7 +67,7 @@ def requires(self) -> list[luigi.Task]:
             RawFileTask(self.project_pedigree_path),
         ]
         if (
-            Env.CHECK_SEX_AND_RELATEDNESS
+            self.check_sex_and_relatedness
             and self.dataset_type.check_sex_and_relatedness
         ):
             requirements = [
@@ -109,7 +112,7 @@ def create_table(self) -> hl.MatrixTable:
         families_failed_relatedness_check = {}
         families_failed_sex_check = {}
         if (
-            Env.CHECK_SEX_AND_RELATEDNESS
+            self.check_sex_and_relatedness
             and self.dataset_type.check_sex_and_relatedness
         ):
             relatedness_check_ht = hl.read_table(self.input()[2].path)

v03_pipeline/lib/tasks/write_remapped_and_subsetted_callset_test.py

@@ -1,6 +1,4 @@
 import shutil
-import unittest
-from unittest.mock import patch
 
 import hail as hl
 import luigi.worker
@@ -72,12 +70,9 @@ def setUp(self) -> None:
             ),
         )
 
-    @patch('v03_pipeline.lib.tasks.write_remapped_and_subsetted_callset.Env')
     def test_write_remapped_and_subsetted_callset_task(
         self,
-        mock_env: unittest.mock.Mock,
     ) -> None:
-        mock_env.CHECK_SEX_AND_RELATEDNESS = True
         worker = luigi.worker.Worker()
         wrsc_task = WriteRemappedAndSubsettedCallsetTask(
             reference_genome=ReferenceGenome.GRCh38,
@@ -88,6 +83,7 @@ def test_write_remapped_and_subsetted_callset_task(
             project_remap_path=TEST_REMAP,
             project_pedigree_path=TEST_PEDIGREE_3,
             validate=False,
+            check_sex_and_relatedness=True,
         )
         worker.add(wrsc_task)
         worker.run()
@@ -108,12 +104,9 @@ def test_write_remapped_and_subsetted_callset_task(
             ],
         )
 
-    @patch('v03_pipeline.lib.tasks.write_remapped_and_subsetted_callset.Env')
     def test_write_remapped_and_subsetted_callset_task_failed_sex_check_family(
         self,
-        mock_env: unittest.mock.Mock,
     ) -> None:
-        mock_env.CHECK_SEX_AND_RELATEDNESS = True
         worker = luigi.worker.Worker()
         wrsc_task = WriteRemappedAndSubsettedCallsetTask(
             reference_genome=ReferenceGenome.GRCh38,
@@ -124,6 +117,7 @@ def test_write_remapped_and_subsetted_callset_task_failed_sex_check_family(
             project_remap_path=TEST_REMAP,
             project_pedigree_path=TEST_PEDIGREE_4,
             validate=False,
+            check_sex_and_relatedness=True,
         )
         worker.add(wrsc_task)
         worker.run()