From 67d65a1fe08a05262abaed3498a4da05574dcb71 Mon Sep 17 00:00:00 2001 From: Ruchit10 Date: Thu, 3 Jul 2025 15:08:15 -0400 Subject: [PATCH 01/48] Add function for exploding intervals to loci --- gnomad/utils/intervals.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index d927b9bbe..236fff774 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -112,3 +112,27 @@ def _add_padding( return [_add_padding(i) for i in intervals] else: return _add_padding(intervals) + + +def explode_intervals_to_loci( + ht: hl.Table, + keep_intervals: bool = False, +) -> hl.Table: + """ + Expand intervals to loci. + + :param ht: Hail Table with an interval field. + :param keep_intervals: If True, keep the original interval as a column in output. + :return: Hail Table keyed by loci and intervals as optional field. + """ + ht = ht.annotate( + pos=hl.range(ht.interval.start.position, ht.interval.end.position + 1), + ).explode("pos") + ht = ht.annotate( + locus=hl.locus( + ht.interval.start.contig, + ht.pos, + reference_genome=ht.interval.start.dtype.reference_genome, + ), + ).key_by("locus") + return ht.drop("interval", "pos") if not keep_intervals else ht.drop("pos") From 9b2cae2781ba0f69bb820b5ed589d4498bc0a994 Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Thu, 3 Jul 2025 17:09:00 -0400 Subject: [PATCH 02/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 1 + 1 file changed, 1 insertion(+) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 236fff774..fbfeb0f01 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -123,6 +123,7 @@ def explode_intervals_to_loci( :param ht: Hail Table with an interval field. :param keep_intervals: If True, keep the original interval as a column in output. + Default is False. :return: Hail Table keyed by loci and intervals as optional field. """ ht = ht.annotate( From 31eeac1cbc302f8d6e336a9aa40c57f14fa17bfb Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Thu, 3 Jul 2025 17:09:39 -0400 Subject: [PATCH 03/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index fbfeb0f01..6a8673924 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -133,7 +133,7 @@ def explode_intervals_to_loci( locus=hl.locus( ht.interval.start.contig, ht.pos, - reference_genome=ht.interval.start.dtype.reference_genome, + reference_genome=ht.interval.start.reference_genome, ), ).key_by("locus") return ht.drop("interval", "pos") if not keep_intervals else ht.drop("pos") From b4214e53a503242af4c4defe6274fa363d638245 Mon Sep 17 00:00:00 2001 From: Ruchit10 Date: Thu, 10 Jul 2025 15:24:10 -0400 Subject: [PATCH 04/48] Add flexibility to accept MT/HT and adjust to interval including start and end --- gnomad/utils/intervals.py | 47 +++++++++++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 12 deletions(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 6a8673924..061edfa18 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -115,25 +115,48 @@ def _add_padding( def explode_intervals_to_loci( - ht: hl.Table, + obj: Union[hl.Table, hl.MatrixTable], + interval_field: str = "interval", keep_intervals: bool = False, -) -> hl.Table: +) -> Union[hl.Table, hl.MatrixTable]: """ Expand intervals to loci. - :param ht: Hail Table with an interval field. + :param obj: Hail Table or MatrixTable with an interval field. + :param interval_field: Name of the interval field. Default is 'interval'. :param keep_intervals: If True, keep the original interval as a column in output. - Default is False. - :return: Hail Table keyed by loci and intervals as optional field. + :return: Hail Table or MatrixTable with interval exploded to loci. """ - ht = ht.annotate( - pos=hl.range(ht.interval.start.position, ht.interval.end.position + 1), - ).explode("pos") + is_matrix = isinstance(obj, hl.MatrixTable) + ht = obj.rows() if is_matrix else obj + + interval = ht[interval_field] + includes_start = interval.includes_start.take(1)[0] + includes_end = interval.includes_end.take(1)[0] + + interval_start = interval.start.position if includes_start else interval.start.position + 1 + interval_end = interval.end.position + 1 if includes_end else interval.end.position + + ht = ht.annotate(pos=hl.range(interval_start, interval_end)).explode("pos") ht = ht.annotate( locus=hl.locus( - ht.interval.start.contig, + ht[interval_field].start.contig, ht.pos, - reference_genome=ht.interval.start.reference_genome, - ), + reference_genome=str(interval.start.take(1)[0].reference_genome) + ) ).key_by("locus") - return ht.drop("interval", "pos") if not keep_intervals else ht.drop("pos") + + fields_to_drop = ["pos"] + if not keep_intervals: + fields_to_drop.append(interval_field) + + ht = ht.drop(*fields_to_drop) + + if is_matrix: + mt = obj + ht = ht.select_globals() + mt = mt.annotate_rows(**ht[mt.row_key]) + mt = mt.filter_rows(hl.is_defined(ht[mt.row_key])) + return mt + else: + return ht From e870d3b619a59c90c8d8037c0cd8bedb909a6e0d Mon Sep 17 00:00:00 2001 From: Ruchit10 Date: Thu, 10 Jul 2025 15:25:29 -0400 Subject: [PATCH 05/48] Black reformatting --- gnomad/utils/intervals.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 061edfa18..4dd4588e3 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -134,7 +134,9 @@ def explode_intervals_to_loci( includes_start = interval.includes_start.take(1)[0] includes_end = interval.includes_end.take(1)[0] - interval_start = interval.start.position if includes_start else interval.start.position + 1 + interval_start = ( + interval.start.position if includes_start else interval.start.position + 1 + ) interval_end = interval.end.position + 1 if includes_end else interval.end.position ht = ht.annotate(pos=hl.range(interval_start, interval_end)).explode("pos") @@ -142,7 +144,7 @@ def explode_intervals_to_loci( locus=hl.locus( ht[interval_field].start.contig, ht.pos, - reference_genome=str(interval.start.take(1)[0].reference_genome) + reference_genome=str(interval.start.take(1)[0].reference_genome), ) ).key_by("locus") From f8d53cb324d511a88e390f42cc10066e22d238c2 Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Fri, 11 Jul 2025 17:14:26 -0400 Subject: [PATCH 06/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 4dd4588e3..621653816 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -115,7 +115,7 @@ def _add_padding( def explode_intervals_to_loci( - obj: Union[hl.Table, hl.MatrixTable], + ht: hl.Table, interval_field: str = "interval", keep_intervals: bool = False, ) -> Union[hl.Table, hl.MatrixTable]: From 3e96a219b9312788a72fbeb0f3096b9f10bf1a3f Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Fri, 11 Jul 2025 17:15:10 -0400 Subject: [PATCH 07/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 621653816..dca794aa9 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -122,7 +122,7 @@ def explode_intervals_to_loci( """ Expand intervals to loci. - :param obj: Hail Table or MatrixTable with an interval field. + :param obj: Hail Table with intervals to be exploded. :param interval_field: Name of the interval field. Default is 'interval'. :param keep_intervals: If True, keep the original interval as a column in output. :return: Hail Table or MatrixTable with interval exploded to loci. From 40e270245360fed4d4312eca05772145b44ba027 Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Fri, 11 Jul 2025 17:15:26 -0400 Subject: [PATCH 08/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index dca794aa9..c8e54145a 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -124,7 +124,7 @@ def explode_intervals_to_loci( :param obj: Hail Table with intervals to be exploded. :param interval_field: Name of the interval field. Default is 'interval'. - :param keep_intervals: If True, keep the original interval as a column in output. + :param keep_intervals: If True, keep the original intervals as a column in output. :return: Hail Table or MatrixTable with interval exploded to loci. """ is_matrix = isinstance(obj, hl.MatrixTable) From 53b50d7256d0ca4de1fcb37697e82bfbfc15a3c7 Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Fri, 11 Jul 2025 17:15:36 -0400 Subject: [PATCH 09/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index c8e54145a..4968a4995 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -125,7 +125,7 @@ def explode_intervals_to_loci( :param obj: Hail Table with intervals to be exploded. :param interval_field: Name of the interval field. Default is 'interval'. :param keep_intervals: If True, keep the original intervals as a column in output. - :return: Hail Table or MatrixTable with interval exploded to loci. + :return: Hail Table with intervals exploded to loci. """ is_matrix = isinstance(obj, hl.MatrixTable) ht = obj.rows() if is_matrix else obj From 3d6774d86f8b70963d967bbfbbfb4493cb2307dd Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Fri, 11 Jul 2025 17:15:49 -0400 Subject: [PATCH 10/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 4968a4995..dafa49ef7 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -130,7 +130,7 @@ def explode_intervals_to_loci( is_matrix = isinstance(obj, hl.MatrixTable) ht = obj.rows() if is_matrix else obj - interval = ht[interval_field] + interval_expr = ht[interval_field] includes_start = interval.includes_start.take(1)[0] includes_end = interval.includes_end.take(1)[0] From 238f5473d67e16fdae18247d239a7fba3e350523 Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Fri, 11 Jul 2025 17:16:30 -0400 Subject: [PATCH 11/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index dafa49ef7..e663f12d5 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -140,13 +140,13 @@ def explode_intervals_to_loci( interval_end = interval.end.position + 1 if includes_end else interval.end.position ht = ht.annotate(pos=hl.range(interval_start, interval_end)).explode("pos") - ht = ht.annotate( + ht = ht.key_by( locus=hl.locus( ht[interval_field].start.contig, ht.pos, - reference_genome=str(interval.start.take(1)[0].reference_genome), + reference_genome=get_reference_genome(ht[interval_field]) ) - ).key_by("locus") + ) fields_to_drop = ["pos"] if not keep_intervals: From 619468848052c521f70626a812dbde0f96ce27bc Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Fri, 11 Jul 2025 17:16:58 -0400 Subject: [PATCH 12/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index e663f12d5..46a9cecee 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -120,7 +120,7 @@ def explode_intervals_to_loci( keep_intervals: bool = False, ) -> Union[hl.Table, hl.MatrixTable]: """ - Expand intervals to loci. + Expand intervals to loci and key by loci. :param obj: Hail Table with intervals to be exploded. :param interval_field: Name of the interval field. Default is 'interval'. From fcf450e576d4d14b05cf2e19302ed94615ba6b09 Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Fri, 11 Jul 2025 17:17:26 -0400 Subject: [PATCH 13/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 46a9cecee..2ae4aebc2 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -118,7 +118,7 @@ def explode_intervals_to_loci( ht: hl.Table, interval_field: str = "interval", keep_intervals: bool = False, -) -> Union[hl.Table, hl.MatrixTable]: +) -> hl.Table, """ Expand intervals to loci and key by loci. From 77c9f2c6320d371843d5f1825d007b516a3e120a Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Fri, 11 Jul 2025 17:17:53 -0400 Subject: [PATCH 14/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 2ae4aebc2..3099c4524 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -127,9 +127,6 @@ def explode_intervals_to_loci( :param keep_intervals: If True, keep the original intervals as a column in output. :return: Hail Table with intervals exploded to loci. """ - is_matrix = isinstance(obj, hl.MatrixTable) - ht = obj.rows() if is_matrix else obj - interval_expr = ht[interval_field] includes_start = interval.includes_start.take(1)[0] includes_end = interval.includes_end.take(1)[0] From caf9dab0b3859034cf7532680d83ae2670cb8057 Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Fri, 11 Jul 2025 17:18:33 -0400 Subject: [PATCH 15/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 3099c4524..3a9951274 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -128,13 +128,8 @@ def explode_intervals_to_loci( :return: Hail Table with intervals exploded to loci. """ interval_expr = ht[interval_field] - includes_start = interval.includes_start.take(1)[0] - includes_end = interval.includes_end.take(1)[0] - - interval_start = ( - interval.start.position if includes_start else interval.start.position + 1 - ) - interval_end = interval.end.position + 1 if includes_end else interval.end.position + interval_start_expr = hl.if_else(interval_expr.includes_start, interval_expr.start.position, interval_expr.start.position + 1) + interval_end_expr = hl.if_else(interval_expr.includes_end, interval_expr.end.position + 1, interval_expr.end.position) ht = ht.annotate(pos=hl.range(interval_start, interval_end)).explode("pos") ht = ht.key_by( From c2dc321b5785400922fbc29b5b9a9613f53290e4 Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Fri, 11 Jul 2025 17:18:56 -0400 Subject: [PATCH 16/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 3a9951274..ec29a830a 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -144,13 +144,5 @@ def explode_intervals_to_loci( if not keep_intervals: fields_to_drop.append(interval_field) - ht = ht.drop(*fields_to_drop) - - if is_matrix: - mt = obj - ht = ht.select_globals() - mt = mt.annotate_rows(**ht[mt.row_key]) - mt = mt.filter_rows(hl.is_defined(ht[mt.row_key])) - return mt - else: - return ht + return ht.drop(*fields_to_drop) + From f4b28a5c6bcf0f3f2849a6678fd67d7c2185b44d Mon Sep 17 00:00:00 2001 From: Ruchit10 Date: Mon, 14 Jul 2025 13:15:46 -0400 Subject: [PATCH 17/48] Change explode_intervals_to_loci to accept interval expression or HT --- gnomad/utils/intervals.py | 71 +++++++++++++++++++++++++++------------ 1 file changed, 49 insertions(+), 22 deletions(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index ec29a830a..139d6046d 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -3,6 +3,7 @@ from typing import List, Union import hail as hl +from gnomad.utils.reference_genome import get_reference_genome def sort_intervals(intervals: List[hl.Interval]): @@ -115,34 +116,60 @@ def _add_padding( def explode_intervals_to_loci( - ht: hl.Table, + obj: Union[hl.Table, hl.expr.IntervalExpression], interval_field: str = "interval", keep_intervals: bool = False, -) -> hl.Table, +) -> Union[hl.Table, hl.expr.ArrayExpression]: """ - Expand intervals to loci and key by loci. + Expand intervals to loci and key by loci, or return loci range expression. - :param obj: Hail Table with intervals to be exploded. - :param interval_field: Name of the interval field. Default is 'interval'. - :param keep_intervals: If True, keep the original intervals as a column in output. - :return: Hail Table with intervals exploded to loci. + :param obj: Hail Table or Interval Expression. + :param interval_field: Name of the interval field if `obj` is a Hail Table. + :param keep_intervals: If True, keep the original intervals as a column in output, if `obj` is a Hail Table. Default is False. + :return: If input is a Hail Table, returns exploded Table keyed by locus. If input is an IntervalExpression, returns position array expression. """ - interval_expr = ht[interval_field] - interval_start_expr = hl.if_else(interval_expr.includes_start, interval_expr.start.position, interval_expr.start.position + 1) - interval_end_expr = hl.if_else(interval_expr.includes_end, interval_expr.end.position + 1, interval_expr.end.position) - - ht = ht.annotate(pos=hl.range(interval_start, interval_end)).explode("pos") - ht = ht.key_by( - locus=hl.locus( - ht[interval_field].start.contig, - ht.pos, - reference_genome=get_reference_genome(ht[interval_field]) + if isinstance(obj, hl.expr.IntervalExpression): + interval = obj + interval_start_expr = hl.if_else( + interval.includes_start, + interval.start.position, + interval.start.position + 1, + ) + interval_end_expr = hl.if_else( + interval.includes_end, interval.end.position + 1, interval.end.position + ) + return hl.range(interval_start_expr, interval_end_expr) + + elif isinstance(obj, hl.Table): + ht = obj + interval_expr = ht[interval_field] + interval_start_expr = hl.if_else( + interval_expr.includes_start, + interval_expr.start.position, + interval_expr.start.position + 1, + ) + interval_end_expr = hl.if_else( + interval_expr.includes_end, + interval_expr.end.position + 1, + interval_expr.end.position, ) - ) - fields_to_drop = ["pos"] - if not keep_intervals: - fields_to_drop.append(interval_field) + ht = ht.annotate(pos=hl.range(interval_start_expr, interval_end_expr)).explode( + "pos" + ) + ht = ht.key_by( + locus=hl.locus( + ht[interval_field].start.contig, + ht.pos, + reference_genome=get_reference_genome(ht[interval_field]), + ) + ) - return ht.drop(*fields_to_drop) + fields_to_drop = ["pos"] + if not keep_intervals: + fields_to_drop.append(interval_field) + return ht.drop(*fields_to_drop) + + else: + raise TypeError("Input must be a Hail Table or a Hail Interval Expression.") From 1162c19092d465940869d2107c4190583d04ce26 Mon Sep 17 00:00:00 2001 From: Ruchit10 Date: Mon, 14 Jul 2025 13:17:51 -0400 Subject: [PATCH 18/48] Fix pre-commit --- gnomad/utils/intervals.py | 1 + 1 file changed, 1 insertion(+) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 139d6046d..3eb987d2e 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -3,6 +3,7 @@ from typing import List, Union import hail as hl + from gnomad.utils.reference_genome import get_reference_genome From 1927ed6c2c326194b3b0fb861cc58a6013f0ea65 Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Tue, 15 Jul 2025 11:13:02 -0400 Subject: [PATCH 19/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 3eb987d2e..1f7db866a 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -118,8 +118,8 @@ def _add_padding( def explode_intervals_to_loci( obj: Union[hl.Table, hl.expr.IntervalExpression], - interval_field: str = "interval", - keep_intervals: bool = False, + interval_field: Optional[str] = None, + keep_intervals: Optional[bool] = None, ) -> Union[hl.Table, hl.expr.ArrayExpression]: """ Expand intervals to loci and key by loci, or return loci range expression. From 91a49486fd594806661b598fbfd3d4a59429483e Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Tue, 15 Jul 2025 11:13:35 -0400 Subject: [PATCH 20/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 1f7db866a..5218394ef 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -125,7 +125,7 @@ def explode_intervals_to_loci( Expand intervals to loci and key by loci, or return loci range expression. :param obj: Hail Table or Interval Expression. - :param interval_field: Name of the interval field if `obj` is a Hail Table. + :param interval_field: Name of the interval field. Only required if input is a Hail Table. Default is None. :param keep_intervals: If True, keep the original intervals as a column in output, if `obj` is a Hail Table. Default is False. :return: If input is a Hail Table, returns exploded Table keyed by locus. If input is an IntervalExpression, returns position array expression. """ From defa0b046c98cdd86456d7dbb36728b7a3d2ecca Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Tue, 15 Jul 2025 11:14:01 -0400 Subject: [PATCH 21/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 5218394ef..4643e36f2 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -126,7 +126,7 @@ def explode_intervals_to_loci( :param obj: Hail Table or Interval Expression. :param interval_field: Name of the interval field. Only required if input is a Hail Table. Default is None. - :param keep_intervals: If True, keep the original intervals as a column in output, if `obj` is a Hail Table. Default is False. + :param keep_intervals: If True, keep the original intervals as a column in output. Only applies if input is a Hail Table. Default is False. :return: If input is a Hail Table, returns exploded Table keyed by locus. If input is an IntervalExpression, returns position array expression. """ if isinstance(obj, hl.expr.IntervalExpression): From 078ac9133b46438d98785f37faeabf14bef4da25 Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Tue, 15 Jul 2025 11:14:32 -0400 Subject: [PATCH 22/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 4643e36f2..03abd8f88 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -117,7 +117,7 @@ def _add_padding( def explode_intervals_to_loci( - obj: Union[hl.Table, hl.expr.IntervalExpression], + intervals: Union[hl.Table, hl.expr.IntervalExpression], interval_field: Optional[str] = None, keep_intervals: Optional[bool] = None, ) -> Union[hl.Table, hl.expr.ArrayExpression]: From 020b493a87817f70cf4d913ea407a9c2e0a037b1 Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Tue, 15 Jul 2025 11:15:50 -0400 Subject: [PATCH 23/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 03abd8f88..153771073 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -129,7 +129,11 @@ def explode_intervals_to_loci( :param keep_intervals: If True, keep the original intervals as a column in output. Only applies if input is a Hail Table. Default is False. :return: If input is a Hail Table, returns exploded Table keyed by locus. If input is an IntervalExpression, returns position array expression. """ - if isinstance(obj, hl.expr.IntervalExpression): + assert isintance(intervals, hl.Table) or isinstance(intervals, hl.expr.IntervalExpression), "Input must be a Table or IntervalExpression!" + + if isinstance(intervals, hl.Table) and (not interval_field or keep_intervals is None): + raise ValueError("`interval_field` and `keep_intervals` must be defined if input is a Table!") + assert interval_field in intervals.row, "`interval_field` must be an annotation present on input Table!" interval = obj interval_start_expr = hl.if_else( interval.includes_start, From 04edebc0da331f437b24a403c70f63ad9629b0ce Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Tue, 15 Jul 2025 11:17:19 -0400 Subject: [PATCH 24/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 153771073..dca931957 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -134,15 +134,17 @@ def explode_intervals_to_loci( if isinstance(intervals, hl.Table) and (not interval_field or keep_intervals is None): raise ValueError("`interval_field` and `keep_intervals` must be defined if input is a Table!") assert interval_field in intervals.row, "`interval_field` must be an annotation present on input Table!" - interval = obj - interval_start_expr = hl.if_else( - interval.includes_start, - interval.start.position, - interval.start.position + 1, - ) - interval_end_expr = hl.if_else( - interval.includes_end, interval.end.position + 1, interval.end.position - ) + intervals_expr = intervals if isinstance(intervals, hl.expr.IntervalExpression) else intervals[interval_field] + intervals_start_expr = hl.if_else( + intervals_expr.includes_start, + intervals_expr.start.position, + intervals_expr.start.position + 1, + ) + intervals_end_expr = hl.if_else( + intervals_expr.includes_end, + intervals_expr.end.position + 1, + intervals_expr.end.position + ) return hl.range(interval_start_expr, interval_end_expr) elif isinstance(obj, hl.Table): From ec2cb971b395c0936a7f440629ba8bec20d27b5b Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Tue, 15 Jul 2025 11:18:53 -0400 Subject: [PATCH 25/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index dca931957..fc6a1325b 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -145,7 +145,26 @@ def explode_intervals_to_loci( intervals_expr.end.position + 1, intervals_expr.end.position ) - return hl.range(interval_start_expr, interval_end_expr) + if isinstance(intervals, hl.Table): + intervals = intervals.annotate(pos=hl.range(intervals_start_expr, intervals_end_expr)).explode( + "pos" + ) + intervals = intervals.key_by( + locus=hl.locus( + intervals[interval_field].start.contig, + intervals.pos, + reference_genome=get_reference_genome(intervals[interval_field]), + ) + ) + + fields_to_drop = ["pos"] + if not keep_intervals: + fields_to_drop.append(interval_field) + + return intervals.drop(*fields_to_drop) + + logger.warning("Input is an IntervalExpression, so function will return ArrayExpression of positions within input intervals. To fully explode intervals to loci, we recommend annotating your dataset with the returned ArrayExpression, exploding the array, and converting the positions to loci!") + return hl.range(intervals_start_expr, intervals_end_expr) elif isinstance(obj, hl.Table): ht = obj From f8864f7852c429501d0c97c81e223227b18be421 Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Tue, 15 Jul 2025 11:19:38 -0400 Subject: [PATCH 26/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index fc6a1325b..b5c1c97fd 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -166,36 +166,3 @@ def explode_intervals_to_loci( logger.warning("Input is an IntervalExpression, so function will return ArrayExpression of positions within input intervals. To fully explode intervals to loci, we recommend annotating your dataset with the returned ArrayExpression, exploding the array, and converting the positions to loci!") return hl.range(intervals_start_expr, intervals_end_expr) - elif isinstance(obj, hl.Table): - ht = obj - interval_expr = ht[interval_field] - interval_start_expr = hl.if_else( - interval_expr.includes_start, - interval_expr.start.position, - interval_expr.start.position + 1, - ) - interval_end_expr = hl.if_else( - interval_expr.includes_end, - interval_expr.end.position + 1, - interval_expr.end.position, - ) - - ht = ht.annotate(pos=hl.range(interval_start_expr, interval_end_expr)).explode( - "pos" - ) - ht = ht.key_by( - locus=hl.locus( - ht[interval_field].start.contig, - ht.pos, - reference_genome=get_reference_genome(ht[interval_field]), - ) - ) - - fields_to_drop = ["pos"] - if not keep_intervals: - fields_to_drop.append(interval_field) - - return ht.drop(*fields_to_drop) - - else: - raise TypeError("Input must be a Hail Table or a Hail Interval Expression.") From 6216cee7467a84ce974ae0022ebce9af13865d05 Mon Sep 17 00:00:00 2001 From: Ruchit10 Date: Tue, 15 Jul 2025 11:41:48 -0400 Subject: [PATCH 27/48] update docstrings, add logging and typing imports, fix typos and indentation --- gnomad/utils/intervals.py | 74 ++++++++++++++++++++++++++------------- 1 file changed, 49 insertions(+), 25 deletions(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index b5c1c97fd..ff8039742 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -1,11 +1,19 @@ # noqa: D100 -from typing import List, Union +import logging +from typing import List, Optional, Union import hail as hl from gnomad.utils.reference_genome import get_reference_genome +logging.basicConfig( + format="%(asctime)s (%(name)s %(lineno)s): %(message)s", + datefmt="%m/%d/%Y %I:%M:%S %p", +) +logger = logging.getLogger(__name__) +logger.setLevel(logging.INFO) + def sort_intervals(intervals: List[hl.Interval]): """ @@ -119,36 +127,48 @@ def _add_padding( def explode_intervals_to_loci( intervals: Union[hl.Table, hl.expr.IntervalExpression], interval_field: Optional[str] = None, - keep_intervals: Optional[bool] = None, + keep_intervals: Optional[bool] = False, ) -> Union[hl.Table, hl.expr.ArrayExpression]: """ Expand intervals to loci and key by loci, or return loci range expression. - :param obj: Hail Table or Interval Expression. + :param intervals: Hail Table or Interval Expression. :param interval_field: Name of the interval field. Only required if input is a Hail Table. Default is None. :param keep_intervals: If True, keep the original intervals as a column in output. Only applies if input is a Hail Table. Default is False. :return: If input is a Hail Table, returns exploded Table keyed by locus. If input is an IntervalExpression, returns position array expression. """ - assert isintance(intervals, hl.Table) or isinstance(intervals, hl.expr.IntervalExpression), "Input must be a Table or IntervalExpression!" - - if isinstance(intervals, hl.Table) and (not interval_field or keep_intervals is None): - raise ValueError("`interval_field` and `keep_intervals` must be defined if input is a Table!") - assert interval_field in intervals.row, "`interval_field` must be an annotation present on input Table!" - intervals_expr = intervals if isinstance(intervals, hl.expr.IntervalExpression) else intervals[interval_field] - intervals_start_expr = hl.if_else( - intervals_expr.includes_start, - intervals_expr.start.position, - intervals_expr.start.position + 1, - ) - intervals_end_expr = hl.if_else( - intervals_expr.includes_end, - intervals_expr.end.position + 1, - intervals_expr.end.position - ) - if isinstance(intervals, hl.Table): - intervals = intervals.annotate(pos=hl.range(intervals_start_expr, intervals_end_expr)).explode( - "pos" + assert isinstance(intervals, hl.Table) or isinstance( + intervals, hl.expr.IntervalExpression + ), "Input must be a Table or IntervalExpression!" + + if isinstance(intervals, hl.Table) and ( + not interval_field or keep_intervals is None + ): + raise ValueError( + "`interval_field` and `keep_intervals` must be defined if input is a Table!" ) + assert ( + interval_field in intervals.row + ), "`interval_field` must be an annotation present on input Table!" + intervals_expr = ( + intervals + if isinstance(intervals, hl.expr.IntervalExpression) + else intervals[interval_field] + ) + intervals_start_expr = hl.if_else( + intervals_expr.includes_start, + intervals_expr.start.position, + intervals_expr.start.position + 1, + ) + intervals_end_expr = hl.if_else( + intervals_expr.includes_end, + intervals_expr.end.position + 1, + intervals_expr.end.position, + ) + if isinstance(intervals, hl.Table): + intervals = intervals.annotate( + pos=hl.range(intervals_start_expr, intervals_end_expr) + ).explode("pos") intervals = intervals.key_by( locus=hl.locus( intervals[interval_field].start.contig, @@ -162,7 +182,11 @@ def explode_intervals_to_loci( fields_to_drop.append(interval_field) return intervals.drop(*fields_to_drop) - - logger.warning("Input is an IntervalExpression, so function will return ArrayExpression of positions within input intervals. To fully explode intervals to loci, we recommend annotating your dataset with the returned ArrayExpression, exploding the array, and converting the positions to loci!") - return hl.range(intervals_start_expr, intervals_end_expr) + logger.warning( + "Input is an IntervalExpression, so function will return ArrayExpression of" + " positions within input intervals. To fully explode intervals to loci, we" + " recommend annotating your dataset with the returned ArrayExpression," + " exploding the array, and converting the positions to loci!" + ) + return hl.range(intervals_start_expr, intervals_end_expr) From 6921adc8ffc1cea93f47fb0a2293fd03d7bcb18e Mon Sep 17 00:00:00 2001 From: Ruchit10 Date: Mon, 2 Feb 2026 17:21:01 -0500 Subject: [PATCH 28/48] Added tests for interval to loci explode function --- tests/utils/test_interval_utils.py | 369 +++++++++++++++++++++++++++++ 1 file changed, 369 insertions(+) create mode 100644 tests/utils/test_interval_utils.py diff --git a/tests/utils/test_interval_utils.py b/tests/utils/test_interval_utils.py new file mode 100644 index 000000000..1461fd426 --- /dev/null +++ b/tests/utils/test_interval_utils.py @@ -0,0 +1,369 @@ +"""Tests for the intervals utility module.""" + +import hail as hl +import pytest + +from gnomad.utils.intervals import explode_intervals_to_loci + + +class TestExplodeIntervalsToLoci: + """Test the explode_intervals_to_loci function.""" + + @pytest.fixture + def sample_interval_table(self): + """Fixture to create a sample Hail Table with intervals.""" + intervals = [ + hl.Interval( + start=hl.Locus("chr1", 100, "GRCh38"), + end=hl.Locus("chr1", 105, "GRCh38"), + includes_start=True, + includes_end=True, + ), + hl.Interval( + start=hl.Locus("chr2", 200, "GRCh38"), + end=hl.Locus("chr2", 203, "GRCh38"), + includes_start=True, + includes_end=False, + ), + ] + return hl.Table.parallelize( + [ + {"interval": intervals[0], "gene": "GENE1"}, + {"interval": intervals[1], "gene": "GENE2"}, + ], + hl.tstruct( + interval=hl.tinterval(hl.tlocus("GRCh38")), + gene=hl.tstr, + ), + ) + + @pytest.fixture + def sample_interval_expr(self): + """Fixture to create a sample interval expression.""" + return hl.Interval( + start=hl.Locus("chr1", 100, "GRCh38"), + end=hl.Locus("chr1", 105, "GRCh38"), + includes_start=True, + includes_end=True, + ) + + def test_explode_table_with_includes_start_and_end( + self, sample_interval_table: hl.Table + ) -> None: + """ + Test exploding a table with intervals that include both start and end positions. + + :param sample_interval_table: Sample Hail Table with intervals. + :return: None. + """ + # Explode the intervals to loci. + result_ht = explode_intervals_to_loci( + sample_interval_table, interval_field="interval", keep_intervals=False + ) + + # Collect results. + result = result_ht.collect() + + # Expected loci for the first interval (100-105, both inclusive). + expected_loci_1 = [ + hl.Locus("chr1", pos, "GRCh38") for pos in range(100, 106) + ] + # Expected loci for the second interval (200-203, start inclusive, end exclusive). + expected_loci_2 = [ + hl.Locus("chr2", pos, "GRCh38") for pos in range(200, 203) + ] + + # Get the loci from the result. + result_loci = [row.locus for row in result] + + # Verify the result contains the expected loci. + assert len(result_loci) == len(expected_loci_1) + len(expected_loci_2) + assert all(locus in result_loci for locus in expected_loci_1) + assert all(locus in result_loci for locus in expected_loci_2) + + def test_explode_table_keep_intervals( + self, sample_interval_table: hl.Table + ) -> None: + """ + Test exploding a table while keeping the original interval field. + + :param sample_interval_table: Sample Hail Table with intervals. + :return: None. + """ + # Explode the intervals to loci, keeping the interval field. + result_ht = explode_intervals_to_loci( + sample_interval_table, interval_field="interval", keep_intervals=True + ) + + # Collect results. + result = result_ht.collect() + + # Verify that the interval field is still present. + assert "interval" in result_ht.row + assert all(hasattr(row, "interval") for row in result) + + # Verify that the locus field is present. + assert "locus" in result_ht.row_key + assert all(hasattr(row, "locus") for row in result) + + def test_explode_table_without_keep_intervals( + self, sample_interval_table: hl.Table + ) -> None: + """ + Test exploding a table without keeping the original interval field. + + :param sample_interval_table: Sample Hail Table with intervals. + :return: None. + """ + # Explode the intervals to loci without keeping the interval field. + result_ht = explode_intervals_to_loci( + sample_interval_table, interval_field="interval", keep_intervals=False + ) + + # Collect results. + result = result_ht.collect() + + # Verify that the interval field is not present. + assert "interval" not in result_ht.row + assert all(not hasattr(row, "interval") for row in result) + + # Verify that other fields are still present. + assert all(hasattr(row, "gene") for row in result) + + def test_explode_interval_expression(self, sample_interval_expr: hl.Interval) -> None: + """ + Test exploding an interval expression to an array of positions. + + :param sample_interval_expr: Sample interval expression. + :return: None. + """ + # Explode the interval expression. + result = explode_intervals_to_loci(sample_interval_expr) + + # Evaluate the result. + positions = hl.eval(result) + + # Expected positions (100-105, both inclusive). + expected_positions = list(range(100, 106)) + + # Verify the result. + assert positions == expected_positions + + def test_explode_interval_expression_excludes_start(self) -> None: + """ + Test exploding an interval expression that excludes the start position. + + :return: None. + """ + interval = hl.Interval( + start=hl.Locus("chr1", 100, "GRCh38"), + end=hl.Locus("chr1", 105, "GRCh38"), + includes_start=False, + includes_end=True, + ) + + # Explode the interval expression. + result = explode_intervals_to_loci(interval) + + # Evaluate the result. + positions = hl.eval(result) + + # Expected positions (101-105, start excluded, end included). + expected_positions = list(range(101, 106)) + + # Verify the result. + assert positions == expected_positions + + def test_explode_interval_expression_excludes_end(self) -> None: + """ + Test exploding an interval expression that excludes the end position. + + :return: None. + """ + interval = hl.Interval( + start=hl.Locus("chr1", 100, "GRCh38"), + end=hl.Locus("chr1", 105, "GRCh38"), + includes_start=True, + includes_end=False, + ) + + # Explode the interval expression. + result = explode_intervals_to_loci(interval) + + # Evaluate the result. + positions = hl.eval(result) + + # Expected positions (100-104, start included, end excluded). + expected_positions = list(range(100, 105)) + + # Verify the result. + assert positions == expected_positions + + def test_explode_interval_expression_excludes_both(self) -> None: + """ + Test exploding an interval expression that excludes both start and end positions. + + :return: None. + """ + interval = hl.Interval( + start=hl.Locus("chr1", 100, "GRCh38"), + end=hl.Locus("chr1", 105, "GRCh38"), + includes_start=False, + includes_end=False, + ) + + # Explode the interval expression. + result = explode_intervals_to_loci(interval) + + # Evaluate the result. + positions = hl.eval(result) + + # Expected positions (101-104, both excluded). + expected_positions = list(range(101, 105)) + + # Verify the result. + assert positions == expected_positions + + def test_explode_table_single_position_interval(self) -> None: + """ + Test exploding a table with a single-position interval. + + :return: None. + """ + interval = hl.Interval( + start=hl.Locus("chr1", 100, "GRCh38"), + end=hl.Locus("chr1", 100, "GRCh38"), + includes_start=True, + includes_end=True, + ) + + ht = hl.Table.parallelize( + [{"interval": interval, "gene": "GENE1"}], + hl.tstruct( + interval=hl.tinterval(hl.tlocus("GRCh38")), + gene=hl.tstr, + ), + ) + + # Explode the intervals to loci. + result_ht = explode_intervals_to_loci( + ht, interval_field="interval", keep_intervals=False + ) + + # Collect results. + result = result_ht.collect() + + # Expected: single locus at position 100. + assert len(result) == 1 + assert result[0].locus == hl.Locus("chr1", 100, "GRCh38") + + def test_explode_table_missing_interval_field_raises_error( + self, sample_interval_table: hl.Table + ) -> None: + """ + Test that an error is raised when interval_field is not provided for a Table. + + :param sample_interval_table: Sample Hail Table with intervals. + :return: None. + """ + with pytest.raises(ValueError, match="`interval_field` and `keep_intervals` must be defined"): + explode_intervals_to_loci(sample_interval_table) + + def test_explode_table_invalid_interval_field_raises_error( + self, sample_interval_table: hl.Table + ) -> None: + """ + Test that an error is raised when interval_field is not in the Table. + + :param sample_interval_table: Sample Hail Table with intervals. + :return: None. + """ + with pytest.raises(AssertionError, match="`interval_field` must be an annotation"): + explode_intervals_to_loci( + sample_interval_table, + interval_field="nonexistent_field", + keep_intervals=False, + ) + + def test_explode_table_grch37(self) -> None: + """ + Test exploding a table with GRCh37 reference genome. + + :return: None. + """ + interval = hl.Interval( + start=hl.Locus("1", 1000, "GRCh37"), + end=hl.Locus("1", 1003, "GRCh37"), + includes_start=True, + includes_end=True, + ) + + ht = hl.Table.parallelize( + [{"interval": interval, "gene": "GENE1"}], + hl.tstruct( + interval=hl.tinterval(hl.tlocus("GRCh37")), + gene=hl.tstr, + ), + ) + + # Explode the intervals to loci. + result_ht = explode_intervals_to_loci( + ht, interval_field="interval", keep_intervals=False + ) + + # Collect results. + result = result_ht.collect() + + # Expected loci (1000-1003, both inclusive). + expected_loci = [hl.Locus("1", pos, "GRCh37") for pos in range(1000, 1004)] + + # Get the loci from the result. + result_loci = [row.locus for row in result] + + # Verify the result. + assert len(result_loci) == len(expected_loci) + assert all(locus in result_loci for locus in expected_loci) + + def test_explode_table_preserves_other_fields(self) -> None: + """ + Test that exploding preserves other fields in the table. + + :return: None. + """ + interval = hl.Interval( + start=hl.Locus("chr1", 100, "GRCh38"), + end=hl.Locus("chr1", 102, "GRCh38"), + includes_start=True, + includes_end=True, + ) + + ht = hl.Table.parallelize( + [{"interval": interval, "gene": "GENE1", "score": 42}], + hl.tstruct( + interval=hl.tinterval(hl.tlocus("GRCh38")), + gene=hl.tstr, + score=hl.tint32, + ), + ) + + # Explode the intervals to loci. + result_ht = explode_intervals_to_loci( + ht, interval_field="interval", keep_intervals=False + ) + + # Collect results. + result = result_ht.collect() + + # Verify that other fields are preserved. + assert all(row.gene == "GENE1" for row in result) + assert all(row.score == 42 for row in result) + assert len(result) == 3 # 3 positions: 100, 101, 102 + + def test_explode_invalid_input_type_raises_error(self) -> None: + """ + Test that an error is raised when input is neither a Table nor an IntervalExpression. + + :return: None. + """ + with pytest.raises(AssertionError, match="Input must be a Table or IntervalExpression"): + explode_intervals_to_loci("invalid_input") From 5f852fcf39c4846a0a3a170463996b7d3a26f009 Mon Sep 17 00:00:00 2001 From: Ruchit10 Date: Fri, 6 Feb 2026 12:55:40 -0500 Subject: [PATCH 29/48] Reformat test script using black --- tests/utils/test_interval_utils.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/tests/utils/test_interval_utils.py b/tests/utils/test_interval_utils.py index 1461fd426..784422e6b 100644 --- a/tests/utils/test_interval_utils.py +++ b/tests/utils/test_interval_utils.py @@ -65,13 +65,9 @@ def test_explode_table_with_includes_start_and_end( result = result_ht.collect() # Expected loci for the first interval (100-105, both inclusive). - expected_loci_1 = [ - hl.Locus("chr1", pos, "GRCh38") for pos in range(100, 106) - ] + expected_loci_1 = [hl.Locus("chr1", pos, "GRCh38") for pos in range(100, 106)] # Expected loci for the second interval (200-203, start inclusive, end exclusive). - expected_loci_2 = [ - hl.Locus("chr2", pos, "GRCh38") for pos in range(200, 203) - ] + expected_loci_2 = [hl.Locus("chr2", pos, "GRCh38") for pos in range(200, 203)] # Get the loci from the result. result_loci = [row.locus for row in result] @@ -130,7 +126,9 @@ def test_explode_table_without_keep_intervals( # Verify that other fields are still present. assert all(hasattr(row, "gene") for row in result) - def test_explode_interval_expression(self, sample_interval_expr: hl.Interval) -> None: + def test_explode_interval_expression( + self, sample_interval_expr: hl.Interval + ) -> None: """ Test exploding an interval expression to an array of positions. @@ -266,7 +264,9 @@ def test_explode_table_missing_interval_field_raises_error( :param sample_interval_table: Sample Hail Table with intervals. :return: None. """ - with pytest.raises(ValueError, match="`interval_field` and `keep_intervals` must be defined"): + with pytest.raises( + ValueError, match="`interval_field` and `keep_intervals` must be defined" + ): explode_intervals_to_loci(sample_interval_table) def test_explode_table_invalid_interval_field_raises_error( @@ -278,7 +278,9 @@ def test_explode_table_invalid_interval_field_raises_error( :param sample_interval_table: Sample Hail Table with intervals. :return: None. """ - with pytest.raises(AssertionError, match="`interval_field` must be an annotation"): + with pytest.raises( + AssertionError, match="`interval_field` must be an annotation" + ): explode_intervals_to_loci( sample_interval_table, interval_field="nonexistent_field", @@ -365,5 +367,7 @@ def test_explode_invalid_input_type_raises_error(self) -> None: :return: None. """ - with pytest.raises(AssertionError, match="Input must be a Table or IntervalExpression"): + with pytest.raises( + AssertionError, match="Input must be a Table or IntervalExpression" + ): explode_intervals_to_loci("invalid_input") From fde24226f1597f0026271073ce36ded59686d0a5 Mon Sep 17 00:00:00 2001 From: Ruchit10 Date: Fri, 24 Apr 2026 13:30:52 -0400 Subject: [PATCH 30/48] Fixed failing tests part 1 --- tests/utils/test_interval_utils.py | 54 ++++++++++++++++++------------ 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/tests/utils/test_interval_utils.py b/tests/utils/test_interval_utils.py index 784422e6b..d3579d456 100644 --- a/tests/utils/test_interval_utils.py +++ b/tests/utils/test_interval_utils.py @@ -40,11 +40,14 @@ def sample_interval_table(self): @pytest.fixture def sample_interval_expr(self): """Fixture to create a sample interval expression.""" - return hl.Interval( - start=hl.Locus("chr1", 100, "GRCh38"), - end=hl.Locus("chr1", 105, "GRCh38"), - includes_start=True, - includes_end=True, + return hl.literal( + hl.Interval( + start=hl.Locus("chr1", 100, "GRCh38"), + end=hl.Locus("chr1", 105, "GRCh38"), + includes_start=True, + includes_end=True, + ), + dtype=hl.tinterval(hl.tlocus("GRCh38")), ) def test_explode_table_with_includes_start_and_end( @@ -99,7 +102,7 @@ def test_explode_table_keep_intervals( assert all(hasattr(row, "interval") for row in result) # Verify that the locus field is present. - assert "locus" in result_ht.row_key + assert "locus" in result_ht.key assert all(hasattr(row, "locus") for row in result) def test_explode_table_without_keep_intervals( @@ -153,11 +156,14 @@ def test_explode_interval_expression_excludes_start(self) -> None: :return: None. """ - interval = hl.Interval( - start=hl.Locus("chr1", 100, "GRCh38"), - end=hl.Locus("chr1", 105, "GRCh38"), - includes_start=False, - includes_end=True, + interval = hl.literal( + hl.Interval( + start=hl.Locus("chr1", 100, "GRCh38"), + end=hl.Locus("chr1", 105, "GRCh38"), + includes_start=False, + includes_end=True, + ), + dtype=hl.tinterval(hl.tlocus("GRCh38")), ) # Explode the interval expression. @@ -178,11 +184,14 @@ def test_explode_interval_expression_excludes_end(self) -> None: :return: None. """ - interval = hl.Interval( - start=hl.Locus("chr1", 100, "GRCh38"), - end=hl.Locus("chr1", 105, "GRCh38"), - includes_start=True, - includes_end=False, + interval = hl.literal( + hl.Interval( + start=hl.Locus("chr1", 100, "GRCh38"), + end=hl.Locus("chr1", 105, "GRCh38"), + includes_start=True, + includes_end=False, + ), + dtype=hl.tinterval(hl.tlocus("GRCh38")), ) # Explode the interval expression. @@ -203,11 +212,14 @@ def test_explode_interval_expression_excludes_both(self) -> None: :return: None. """ - interval = hl.Interval( - start=hl.Locus("chr1", 100, "GRCh38"), - end=hl.Locus("chr1", 105, "GRCh38"), - includes_start=False, - includes_end=False, + interval = hl.literal( + hl.Interval( + start=hl.Locus("chr1", 100, "GRCh38"), + end=hl.Locus("chr1", 105, "GRCh38"), + includes_start=False, + includes_end=False, + ), + dtype=hl.tinterval(hl.tlocus("GRCh38")), ) # Explode the interval expression. From ccd07fbf534e5ba0bda592305638f9101fd17ae9 Mon Sep 17 00:00:00 2001 From: Ruchit10 Date: Fri, 24 Apr 2026 14:59:15 -0400 Subject: [PATCH 31/48] Fixed intervals assertion in explode_intervals_to_loci --- gnomad/utils/intervals.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index ff8039742..d1662b4ca 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -147,9 +147,10 @@ def explode_intervals_to_loci( raise ValueError( "`interval_field` and `keep_intervals` must be defined if input is a Table!" ) - assert ( - interval_field in intervals.row - ), "`interval_field` must be an annotation present on input Table!" + if isinstance(intervals, hl.Table): + assert ( + interval_field in intervals.row + ), "`interval_field` must be an annotation present on input Table!" intervals_expr = ( intervals if isinstance(intervals, hl.expr.IntervalExpression) From 31a307b87075f2fdd0345793ebb7ebbbe84d8f7d Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Fri, 24 Apr 2026 17:52:27 -0400 Subject: [PATCH 32/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index d1662b4ca..07c9dbfba 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -132,7 +132,7 @@ def explode_intervals_to_loci( """ Expand intervals to loci and key by loci, or return loci range expression. - :param intervals: Hail Table or Interval Expression. + :param intervals: Table or IntervalExpression. :param interval_field: Name of the interval field. Only required if input is a Hail Table. Default is None. :param keep_intervals: If True, keep the original intervals as a column in output. Only applies if input is a Hail Table. Default is False. :return: If input is a Hail Table, returns exploded Table keyed by locus. If input is an IntervalExpression, returns position array expression. From de65275ee0344797a287135f4994c77671f9f6fd Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Fri, 24 Apr 2026 17:53:09 -0400 Subject: [PATCH 33/48] Update gnomad/utils/intervals.py Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 07c9dbfba..2a3b384d3 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -135,7 +135,7 @@ def explode_intervals_to_loci( :param intervals: Table or IntervalExpression. :param interval_field: Name of the interval field. Only required if input is a Hail Table. Default is None. :param keep_intervals: If True, keep the original intervals as a column in output. Only applies if input is a Hail Table. Default is False. - :return: If input is a Hail Table, returns exploded Table keyed by locus. If input is an IntervalExpression, returns position array expression. + :return: If input is a Hail Table, returns exploded Table keyed by locus. If input is an IntervalExpression, returns ArrayExpression containing loci within input interval. """ assert isinstance(intervals, hl.Table) or isinstance( intervals, hl.expr.IntervalExpression From 9b6b52d2a99b574c3a34f4802f289cb2ffcfe625 Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Fri, 24 Apr 2026 18:09:40 -0400 Subject: [PATCH 34/48] Apply suggestions from code review Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 2a3b384d3..7ea1ef6d5 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -168,25 +168,25 @@ def explode_intervals_to_loci( ) if isinstance(intervals, hl.Table): intervals = intervals.annotate( - pos=hl.range(intervals_start_expr, intervals_end_expr) - ).explode("pos") + _pos=hl.range(intervals_start_expr, intervals_end_expr) + ).explode("_pos") intervals = intervals.key_by( locus=hl.locus( intervals[interval_field].start.contig, - intervals.pos, + intervals._pos, reference_genome=get_reference_genome(intervals[interval_field]), ) ) - fields_to_drop = ["pos"] + fields_to_drop = ["_pos"] if not keep_intervals: fields_to_drop.append(interval_field) return intervals.drop(*fields_to_drop) - logger.warning( + logger.info( "Input is an IntervalExpression, so function will return ArrayExpression of" - " positions within input intervals. To fully explode intervals to loci, we" + " positions within input intervals. To fully explode intervals to loci, we" " recommend annotating your dataset with the returned ArrayExpression," " exploding the array, and converting the positions to loci!" ) From 25be488187eebafb12c6a59effa7ee8e2b9523c1 Mon Sep 17 00:00:00 2001 From: Ruchit10 Date: Mon, 27 Apr 2026 15:53:18 -0400 Subject: [PATCH 35/48] Changes addressed per PR comments and added more tests --- gnomad/utils/intervals.py | 91 ++++++- tests/utils/test_interval_utils.py | 385 +++++++++++++++++++++-------- 2 files changed, 358 insertions(+), 118 deletions(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 7ea1ef6d5..7f50cebc1 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -125,21 +125,41 @@ def _add_padding( def explode_intervals_to_loci( - intervals: Union[hl.Table, hl.expr.IntervalExpression], + intervals: Union[ + hl.Table, hl.expr.IntervalExpression, List[hl.expr.IntervalExpression] + ], interval_field: Optional[str] = None, keep_intervals: Optional[bool] = False, + deduplicate: bool = True, ) -> Union[hl.Table, hl.expr.ArrayExpression]: """ - Expand intervals to loci and key by loci, or return loci range expression. + Expand interval(s) to loci. + + If input is a Table, function will expand intervals to loci and key Table by loci. + If input is an IntervalExpression or a list of IntervalExpressions, function will return an ArrayExpression containing all loci within the input interval(s). + + .. warning:: + - Overlapping intervals will produce duplicate loci. Use ``deduplicate=True`` (the default) to remove them. + - When ``keep_intervals=True`` on a Table input, deduplication is not possible because duplicate rows with different interval annotations may exist; a warning is displayed instead. + - Caution when using this function on very larg intervals (e.g. whole chromosomes), as it will create extremely large arrays, which may cause performance issues. + + NOTE: Intervals that cross chromosomes is currently not supported. - :param intervals: Table or IntervalExpression. + :param intervals: Table, IntervalExpression, or list of IntervalExpressions. :param interval_field: Name of the interval field. Only required if input is a Hail Table. Default is None. :param keep_intervals: If True, keep the original intervals as a column in output. Only applies if input is a Hail Table. Default is False. - :return: If input is a Hail Table, returns exploded Table keyed by locus. If input is an IntervalExpression, returns ArrayExpression containing loci within input interval. + :param deduplicate: If True, remove duplicate loci produced by overlapping intervals. For Table input with ``keep_intervals=True``, deduplication is skipped with a warning. + For a list of IntervalExpressions, the returned ArrayExpression will have duplicate positions removed. Default is True. + :return: If input is a Hail Table, returns exploded Table keyed by locus. If input is an IntervalExpression or list of IntervalExpressions, returns ArrayExpression containing loci within input interval(s). """ - assert isinstance(intervals, hl.Table) or isinstance( - intervals, hl.expr.IntervalExpression - ), "Input must be a Table or IntervalExpression!" + assert ( + isinstance(intervals, hl.Table) + or isinstance(intervals, hl.expr.IntervalExpression) + or ( + isinstance(intervals, list) + and all(isinstance(i, hl.expr.IntervalExpression) for i in intervals) + ) + ), "Input must be a Table, IntervalExpression, or list of IntervalExpressions!" if isinstance(intervals, hl.Table) and ( not interval_field or keep_intervals is None @@ -148,9 +168,45 @@ def explode_intervals_to_loci( "`interval_field` and `keep_intervals` must be defined if input is a Table!" ) if isinstance(intervals, hl.Table): - assert ( - interval_field in intervals.row - ), "`interval_field` must be an annotation present on input Table!" + if interval_field not in intervals.row: + raise ValueError( + "`interval_field` must be an annotation present on input Table!" + ) + + if isinstance(intervals, list): + logger.info( + "Input is a list of IntervalExpressions, so function will return an" + " ArrayExpression of positions within all input intervals. To fully explode" + " intervals to loci, we recommend annotating your dataset with the returned" + " ArrayExpression, exploding the array, and converting the positions to" + " loci!" + ) + if not deduplicate: + logger.warning( + "Overlapping intervals in the input list may produce duplicate loci in" + " the returned ArrayExpression. Set `deduplicate=True` to remove them." + ) + + def _interval_to_range(interval_expr): + start = hl.if_else( + interval_expr.includes_start, + interval_expr.start.position, + interval_expr.start.position + 1, + ) + end = hl.if_else( + interval_expr.includes_end, + interval_expr.end.position + 1, + interval_expr.end.position, + ) + return hl.range(start, end) + + result = hl.array([_interval_to_range(i) for i in intervals]).flatmap( + lambda x: x + ) + if deduplicate: + result = hl.array(hl.set(result)) + return result + intervals_expr = ( intervals if isinstance(intervals, hl.expr.IntervalExpression) @@ -182,7 +238,20 @@ def explode_intervals_to_loci( if not keep_intervals: fields_to_drop.append(interval_field) - return intervals.drop(*fields_to_drop) + intervals = intervals.drop(*fields_to_drop) + + if deduplicate: + if keep_intervals: + logger.warning( + "`deduplicate=True` has no effect when `keep_intervals=True`" + " because rows with different interval annotations cannot be safely" + " collapsed. Duplicate loci may be present in the output if" + " intervals overlap." + ) + else: + intervals = intervals.distinct() + + return intervals logger.info( "Input is an IntervalExpression, so function will return ArrayExpression of" diff --git a/tests/utils/test_interval_utils.py b/tests/utils/test_interval_utils.py index d3579d456..06299bb9c 100644 --- a/tests/utils/test_interval_utils.py +++ b/tests/utils/test_interval_utils.py @@ -40,25 +40,16 @@ def sample_interval_table(self): @pytest.fixture def sample_interval_expr(self): """Fixture to create a sample interval expression.""" - return hl.literal( - hl.Interval( - start=hl.Locus("chr1", 100, "GRCh38"), - end=hl.Locus("chr1", 105, "GRCh38"), - includes_start=True, - includes_end=True, - ), - dtype=hl.tinterval(hl.tlocus("GRCh38")), + return hl.interval( + hl.locus("chr1", 100, "GRCh38"), + hl.locus("chr1", 105, "GRCh38"), + includes_start=True, + includes_end=True, ) def test_explode_table_with_includes_start_and_end( self, sample_interval_table: hl.Table ) -> None: - """ - Test exploding a table with intervals that include both start and end positions. - - :param sample_interval_table: Sample Hail Table with intervals. - :return: None. - """ # Explode the intervals to loci. result_ht = explode_intervals_to_loci( sample_interval_table, interval_field="interval", keep_intervals=False @@ -83,12 +74,6 @@ def test_explode_table_with_includes_start_and_end( def test_explode_table_keep_intervals( self, sample_interval_table: hl.Table ) -> None: - """ - Test exploding a table while keeping the original interval field. - - :param sample_interval_table: Sample Hail Table with intervals. - :return: None. - """ # Explode the intervals to loci, keeping the interval field. result_ht = explode_intervals_to_loci( sample_interval_table, interval_field="interval", keep_intervals=True @@ -108,12 +93,6 @@ def test_explode_table_keep_intervals( def test_explode_table_without_keep_intervals( self, sample_interval_table: hl.Table ) -> None: - """ - Test exploding a table without keeping the original interval field. - - :param sample_interval_table: Sample Hail Table with intervals. - :return: None. - """ # Explode the intervals to loci without keeping the interval field. result_ht = explode_intervals_to_loci( sample_interval_table, interval_field="interval", keep_intervals=False @@ -130,14 +109,8 @@ def test_explode_table_without_keep_intervals( assert all(hasattr(row, "gene") for row in result) def test_explode_interval_expression( - self, sample_interval_expr: hl.Interval + self, sample_interval_expr: hl.expr.IntervalExpression ) -> None: - """ - Test exploding an interval expression to an array of positions. - - :param sample_interval_expr: Sample interval expression. - :return: None. - """ # Explode the interval expression. result = explode_intervals_to_loci(sample_interval_expr) @@ -151,19 +124,11 @@ def test_explode_interval_expression( assert positions == expected_positions def test_explode_interval_expression_excludes_start(self) -> None: - """ - Test exploding an interval expression that excludes the start position. - - :return: None. - """ - interval = hl.literal( - hl.Interval( - start=hl.Locus("chr1", 100, "GRCh38"), - end=hl.Locus("chr1", 105, "GRCh38"), - includes_start=False, - includes_end=True, - ), - dtype=hl.tinterval(hl.tlocus("GRCh38")), + interval = hl.interval( + hl.locus("chr1", 100, "GRCh38"), + hl.locus("chr1", 105, "GRCh38"), + includes_start=False, + includes_end=True, ) # Explode the interval expression. @@ -179,19 +144,11 @@ def test_explode_interval_expression_excludes_start(self) -> None: assert positions == expected_positions def test_explode_interval_expression_excludes_end(self) -> None: - """ - Test exploding an interval expression that excludes the end position. - - :return: None. - """ - interval = hl.literal( - hl.Interval( - start=hl.Locus("chr1", 100, "GRCh38"), - end=hl.Locus("chr1", 105, "GRCh38"), - includes_start=True, - includes_end=False, - ), - dtype=hl.tinterval(hl.tlocus("GRCh38")), + interval = hl.interval( + hl.locus("chr1", 100, "GRCh38"), + hl.locus("chr1", 105, "GRCh38"), + includes_start=True, + includes_end=False, ) # Explode the interval expression. @@ -207,19 +164,11 @@ def test_explode_interval_expression_excludes_end(self) -> None: assert positions == expected_positions def test_explode_interval_expression_excludes_both(self) -> None: - """ - Test exploding an interval expression that excludes both start and end positions. - - :return: None. - """ - interval = hl.literal( - hl.Interval( - start=hl.Locus("chr1", 100, "GRCh38"), - end=hl.Locus("chr1", 105, "GRCh38"), - includes_start=False, - includes_end=False, - ), - dtype=hl.tinterval(hl.tlocus("GRCh38")), + interval = hl.interval( + hl.locus("chr1", 100, "GRCh38"), + hl.locus("chr1", 105, "GRCh38"), + includes_start=False, + includes_end=False, ) # Explode the interval expression. @@ -235,12 +184,7 @@ def test_explode_interval_expression_excludes_both(self) -> None: assert positions == expected_positions def test_explode_table_single_position_interval(self) -> None: - """ - Test exploding a table with a single-position interval. - - :return: None. - """ - interval = hl.Interval( + interval = hl.interval( start=hl.Locus("chr1", 100, "GRCh38"), end=hl.Locus("chr1", 100, "GRCh38"), includes_start=True, @@ -270,12 +214,6 @@ def test_explode_table_single_position_interval(self) -> None: def test_explode_table_missing_interval_field_raises_error( self, sample_interval_table: hl.Table ) -> None: - """ - Test that an error is raised when interval_field is not provided for a Table. - - :param sample_interval_table: Sample Hail Table with intervals. - :return: None. - """ with pytest.raises( ValueError, match="`interval_field` and `keep_intervals` must be defined" ): @@ -284,12 +222,6 @@ def test_explode_table_missing_interval_field_raises_error( def test_explode_table_invalid_interval_field_raises_error( self, sample_interval_table: hl.Table ) -> None: - """ - Test that an error is raised when interval_field is not in the Table. - - :param sample_interval_table: Sample Hail Table with intervals. - :return: None. - """ with pytest.raises( AssertionError, match="`interval_field` must be an annotation" ): @@ -300,12 +232,7 @@ def test_explode_table_invalid_interval_field_raises_error( ) def test_explode_table_grch37(self) -> None: - """ - Test exploding a table with GRCh37 reference genome. - - :return: None. - """ - interval = hl.Interval( + interval = hl.interval( start=hl.Locus("1", 1000, "GRCh37"), end=hl.Locus("1", 1003, "GRCh37"), includes_start=True, @@ -339,12 +266,7 @@ def test_explode_table_grch37(self) -> None: assert all(locus in result_loci for locus in expected_loci) def test_explode_table_preserves_other_fields(self) -> None: - """ - Test that exploding preserves other fields in the table. - - :return: None. - """ - interval = hl.Interval( + interval = hl.interval( start=hl.Locus("chr1", 100, "GRCh38"), end=hl.Locus("chr1", 102, "GRCh38"), includes_start=True, @@ -374,12 +296,261 @@ def test_explode_table_preserves_other_fields(self) -> None: assert len(result) == 3 # 3 positions: 100, 101, 102 def test_explode_invalid_input_type_raises_error(self) -> None: - """ - Test that an error is raised when input is neither a Table nor an IntervalExpression. - - :return: None. - """ with pytest.raises( AssertionError, match="Input must be a Table or IntervalExpression" ): explode_intervals_to_loci("invalid_input") + + def test_explode_interval_expression_single_position_excludes_both(self) -> None: + interval = hl.interval( + hl.locus("chr1", 100, "GRCh38"), + hl.locus("chr1", 100, "GRCh38"), + includes_start=False, + includes_end=False, + ) + + result = explode_intervals_to_loci(interval) + positions = hl.eval(result) + + assert positions == [] + + def test_explode_table_single_position_interval_excludes_both(self) -> None: + interval = hl.interval( + start=hl.Locus("chr1", 100, "GRCh38"), + end=hl.Locus("chr1", 100, "GRCh38"), + includes_start=False, + includes_end=False, + ) + + ht = hl.Table.parallelize( + [{"interval": interval, "gene": "GENE1"}], + hl.tstruct(interval=hl.tinterval(hl.tlocus("GRCh38")), gene=hl.tstr), + ) + + result_ht = explode_intervals_to_loci( + ht, interval_field="interval", keep_intervals=False + ) + + assert result_ht.count() == 0 + + def test_explode_table_excludes_start(self) -> None: + interval = hl.interval( + start=hl.Locus("chr1", 100, "GRCh38"), + end=hl.Locus("chr1", 105, "GRCh38"), + includes_start=False, + includes_end=True, + ) + + ht = hl.Table.parallelize( + [{"interval": interval, "gene": "GENE1"}], + hl.tstruct(interval=hl.tinterval(hl.tlocus("GRCh38")), gene=hl.tstr), + ) + + result_ht = explode_intervals_to_loci( + ht, interval_field="interval", keep_intervals=False + ) + result_loci = [row.locus for row in result_ht.collect()] + + expected_loci = [hl.Locus("chr1", pos, "GRCh38") for pos in range(101, 106)] + assert len(result_loci) == len(expected_loci) + assert all(locus in result_loci for locus in expected_loci) + + def test_explode_table_excludes_end(self) -> None: + interval = hl.interval( + start=hl.Locus("chr1", 100, "GRCh38"), + end=hl.Locus("chr1", 105, "GRCh38"), + includes_start=True, + includes_end=False, + ) + + ht = hl.Table.parallelize( + [{"interval": interval, "gene": "GENE1"}], + hl.tstruct(interval=hl.tinterval(hl.tlocus("GRCh38")), gene=hl.tstr), + ) + + result_ht = explode_intervals_to_loci( + ht, interval_field="interval", keep_intervals=False + ) + result_loci = [row.locus for row in result_ht.collect()] + + expected_loci = [hl.Locus("chr1", pos, "GRCh38") for pos in range(100, 105)] + assert len(result_loci) == len(expected_loci) + assert all(locus in result_loci for locus in expected_loci) + + def test_explode_table_excludes_both(self) -> None: + interval = hl.interval( + start=hl.Locus("chr1", 100, "GRCh38"), + end=hl.Locus("chr1", 105, "GRCh38"), + includes_start=False, + includes_end=False, + ) + + ht = hl.Table.parallelize( + [{"interval": interval, "gene": "GENE1"}], + hl.tstruct(interval=hl.tinterval(hl.tlocus("GRCh38")), gene=hl.tstr), + ) + + result_ht = explode_intervals_to_loci( + ht, interval_field="interval", keep_intervals=False + ) + result_loci = [row.locus for row in result_ht.collect()] + + expected_loci = [hl.Locus("chr1", pos, "GRCh38") for pos in range(101, 105)] + assert len(result_loci) == len(expected_loci) + assert all(locus in result_loci for locus in expected_loci) + + def test_explode_list_of_interval_expressions(self) -> None: + intervals = [ + hl.interval( + hl.locus("chr1", 100, "GRCh38"), + hl.locus("chr1", 102, "GRCh38"), + includes_start=True, + includes_end=True, + ), + hl.interval( + hl.locus("chr2", 200, "GRCh38"), + hl.locus("chr2", 202, "GRCh38"), + includes_start=True, + includes_end=False, + ), + ] + + result = explode_intervals_to_loci(intervals) + positions = hl.eval(result) + + # deduplicate=True by default; set comparison since order is not guaranteed. + assert set(positions) == {100, 101, 102, 200, 201} + + def test_explode_table_overlapping_intervals_deduplicates(self) -> None: + # chr1:100-105 and chr1:103-108, both inclusive → without dedup: 12 rows + # with deduplicate=True (default): 9 distinct loci [100..108] + intervals = [ + hl.interval( + start=hl.Locus("chr1", 100, "GRCh38"), + end=hl.Locus("chr1", 105, "GRCh38"), + includes_start=True, + includes_end=True, + ), + hl.interval( + start=hl.Locus("chr1", 103, "GRCh38"), + end=hl.Locus("chr1", 108, "GRCh38"), + includes_start=True, + includes_end=True, + ), + ] + + ht = hl.Table.parallelize( + [ + {"interval": intervals[0], "gene": "GENE1"}, + {"interval": intervals[1], "gene": "GENE2"}, + ], + hl.tstruct(interval=hl.tinterval(hl.tlocus("GRCh38")), gene=hl.tstr), + ) + + result_ht = explode_intervals_to_loci( + ht, interval_field="interval", keep_intervals=False, deduplicate=True + ) + result_loci = [row.locus for row in result_ht.collect()] + + expected_loci = [hl.Locus("chr1", pos, "GRCh38") for pos in range(100, 109)] + assert len(result_loci) == len(expected_loci) + assert all(locus in result_loci for locus in expected_loci) + + def test_explode_table_overlapping_intervals_keep_intervals_warns( + self, caplog + ) -> None: + # With keep_intervals=True and deduplicate=True, deduplication is skipped and + # a warning is emitted; overlapping positions appear as duplicate rows. + import logging + + intervals = [ + hl.interval( + start=hl.Locus("chr1", 100, "GRCh38"), + end=hl.Locus("chr1", 102, "GRCh38"), + includes_start=True, + includes_end=True, + ), + hl.interval( + start=hl.Locus("chr1", 101, "GRCh38"), + end=hl.Locus("chr1", 103, "GRCh38"), + includes_start=True, + includes_end=True, + ), + ] + + ht = hl.Table.parallelize( + [ + {"interval": intervals[0], "gene": "GENE1"}, + {"interval": intervals[1], "gene": "GENE2"}, + ], + hl.tstruct(interval=hl.tinterval(hl.tlocus("GRCh38")), gene=hl.tstr), + ) + + with caplog.at_level(logging.WARNING): + result_ht = explode_intervals_to_loci( + ht, interval_field="interval", keep_intervals=True, deduplicate=True + ) + + assert ( + "`deduplicate=True` has no effect when `keep_intervals=True`" in caplog.text + ) + # Duplicate loci are present: positions 101 and 102 each appear twice. + result_loci = [row.locus for row in result_ht.collect()] + assert ( + len(result_loci) == 7 + ) # 3 from GENE1 + 3 from GENE2, positions 101-102 duplicated + + def test_explode_list_overlapping_intervals_deduplicates(self) -> None: + # Two overlapping intervals; deduplicate=True (default) removes duplicate positions. + intervals = [ + hl.interval( + hl.locus("chr1", 100, "GRCh38"), + hl.locus("chr1", 105, "GRCh38"), + includes_start=True, + includes_end=True, + ), + hl.interval( + hl.locus("chr1", 103, "GRCh38"), + hl.locus("chr1", 108, "GRCh38"), + includes_start=True, + includes_end=True, + ), + ] + + result = explode_intervals_to_loci(intervals, deduplicate=True) + positions = hl.eval(result) + + assert set(positions) == set(range(100, 109)) + + def test_explode_list_overlapping_intervals_no_deduplicate_warns( + self, caplog + ) -> None: + import logging + + intervals = [ + hl.interval( + hl.locus("chr1", 100, "GRCh38"), + hl.locus("chr1", 102, "GRCh38"), + includes_start=True, + includes_end=True, + ), + hl.interval( + hl.locus("chr1", 101, "GRCh38"), + hl.locus("chr1", 103, "GRCh38"), + includes_start=True, + includes_end=True, + ), + ] + + with caplog.at_level(logging.WARNING): + result = explode_intervals_to_loci(intervals, deduplicate=False) + + assert ( + "Overlapping intervals in the input list may produce duplicate loci" + in caplog.text + ) + positions = hl.eval(result) + # Positions 101 and 102 appear twice due to overlap. + assert len(positions) == 7 + assert positions.count(101) == 2 + assert positions.count(102) == 2 From 940a220c5945714110d1a8567efcb9cb71b4d6a0 Mon Sep 17 00:00:00 2001 From: Ruchit10 Date: Mon, 27 Apr 2026 16:14:27 -0400 Subject: [PATCH 36/48] Add back doc strings in test functions to fix pylint errors --- tests/utils/test_interval_utils.py | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/tests/utils/test_interval_utils.py b/tests/utils/test_interval_utils.py index 06299bb9c..8ae735a52 100644 --- a/tests/utils/test_interval_utils.py +++ b/tests/utils/test_interval_utils.py @@ -50,6 +50,7 @@ def sample_interval_expr(self): def test_explode_table_with_includes_start_and_end( self, sample_interval_table: hl.Table ) -> None: + """Test exploding a table with intervals that include both start and end positions.""" # Explode the intervals to loci. result_ht = explode_intervals_to_loci( sample_interval_table, interval_field="interval", keep_intervals=False @@ -60,7 +61,8 @@ def test_explode_table_with_includes_start_and_end( # Expected loci for the first interval (100-105, both inclusive). expected_loci_1 = [hl.Locus("chr1", pos, "GRCh38") for pos in range(100, 106)] - # Expected loci for the second interval (200-203, start inclusive, end exclusive). + # Expected loci for the second interval (200-203, start inclusive, end + # exclusive). expected_loci_2 = [hl.Locus("chr2", pos, "GRCh38") for pos in range(200, 203)] # Get the loci from the result. @@ -74,6 +76,7 @@ def test_explode_table_with_includes_start_and_end( def test_explode_table_keep_intervals( self, sample_interval_table: hl.Table ) -> None: + """Test that the original interval field is retained when keep_intervals=True.""" # Explode the intervals to loci, keeping the interval field. result_ht = explode_intervals_to_loci( sample_interval_table, interval_field="interval", keep_intervals=True @@ -93,6 +96,7 @@ def test_explode_table_keep_intervals( def test_explode_table_without_keep_intervals( self, sample_interval_table: hl.Table ) -> None: + """Test that the interval field is dropped when keep_intervals=False.""" # Explode the intervals to loci without keeping the interval field. result_ht = explode_intervals_to_loci( sample_interval_table, interval_field="interval", keep_intervals=False @@ -111,6 +115,7 @@ def test_explode_table_without_keep_intervals( def test_explode_interval_expression( self, sample_interval_expr: hl.expr.IntervalExpression ) -> None: + """Test exploding a single IntervalExpression returns the correct positions.""" # Explode the interval expression. result = explode_intervals_to_loci(sample_interval_expr) @@ -124,6 +129,7 @@ def test_explode_interval_expression( assert positions == expected_positions def test_explode_interval_expression_excludes_start(self) -> None: + """Test that the start position is excluded when includes_start=False.""" interval = hl.interval( hl.locus("chr1", 100, "GRCh38"), hl.locus("chr1", 105, "GRCh38"), @@ -144,6 +150,7 @@ def test_explode_interval_expression_excludes_start(self) -> None: assert positions == expected_positions def test_explode_interval_expression_excludes_end(self) -> None: + """Test that the end position is excluded when includes_end=False.""" interval = hl.interval( hl.locus("chr1", 100, "GRCh38"), hl.locus("chr1", 105, "GRCh38"), @@ -164,6 +171,7 @@ def test_explode_interval_expression_excludes_end(self) -> None: assert positions == expected_positions def test_explode_interval_expression_excludes_both(self) -> None: + """Test that both endpoints are excluded when includes_start and includes_end are False.""" interval = hl.interval( hl.locus("chr1", 100, "GRCh38"), hl.locus("chr1", 105, "GRCh38"), @@ -184,6 +192,7 @@ def test_explode_interval_expression_excludes_both(self) -> None: assert positions == expected_positions def test_explode_table_single_position_interval(self) -> None: + """Test exploding a table with a single-position interval that includes both endpoints.""" interval = hl.interval( start=hl.Locus("chr1", 100, "GRCh38"), end=hl.Locus("chr1", 100, "GRCh38"), @@ -214,6 +223,7 @@ def test_explode_table_single_position_interval(self) -> None: def test_explode_table_missing_interval_field_raises_error( self, sample_interval_table: hl.Table ) -> None: + """Test that a ValueError is raised when interval_field is not provided for a Table.""" with pytest.raises( ValueError, match="`interval_field` and `keep_intervals` must be defined" ): @@ -222,6 +232,7 @@ def test_explode_table_missing_interval_field_raises_error( def test_explode_table_invalid_interval_field_raises_error( self, sample_interval_table: hl.Table ) -> None: + """Test that an error is raised when interval_field is not present in the Table.""" with pytest.raises( AssertionError, match="`interval_field` must be an annotation" ): @@ -232,6 +243,7 @@ def test_explode_table_invalid_interval_field_raises_error( ) def test_explode_table_grch37(self) -> None: + """Test exploding a table with GRCh37 reference genome intervals.""" interval = hl.interval( start=hl.Locus("1", 1000, "GRCh37"), end=hl.Locus("1", 1003, "GRCh37"), @@ -266,6 +278,7 @@ def test_explode_table_grch37(self) -> None: assert all(locus in result_loci for locus in expected_loci) def test_explode_table_preserves_other_fields(self) -> None: + """Test that non-interval fields are preserved after exploding.""" interval = hl.interval( start=hl.Locus("chr1", 100, "GRCh38"), end=hl.Locus("chr1", 102, "GRCh38"), @@ -296,12 +309,14 @@ def test_explode_table_preserves_other_fields(self) -> None: assert len(result) == 3 # 3 positions: 100, 101, 102 def test_explode_invalid_input_type_raises_error(self) -> None: + """Test that an AssertionError is raised when input is an unsupported type.""" with pytest.raises( AssertionError, match="Input must be a Table or IntervalExpression" ): explode_intervals_to_loci("invalid_input") def test_explode_interval_expression_single_position_excludes_both(self) -> None: + """Test that a single-position interval with both endpoints excluded returns an empty array.""" interval = hl.interval( hl.locus("chr1", 100, "GRCh38"), hl.locus("chr1", 100, "GRCh38"), @@ -315,6 +330,7 @@ def test_explode_interval_expression_single_position_excludes_both(self) -> None assert positions == [] def test_explode_table_single_position_interval_excludes_both(self) -> None: + """Test that a table with a single-position interval excluding both endpoints returns 0 rows.""" interval = hl.interval( start=hl.Locus("chr1", 100, "GRCh38"), end=hl.Locus("chr1", 100, "GRCh38"), @@ -334,6 +350,7 @@ def test_explode_table_single_position_interval_excludes_both(self) -> None: assert result_ht.count() == 0 def test_explode_table_excludes_start(self) -> None: + """Test that the start position is excluded when exploding a table interval with includes_start=False.""" interval = hl.interval( start=hl.Locus("chr1", 100, "GRCh38"), end=hl.Locus("chr1", 105, "GRCh38"), @@ -356,6 +373,7 @@ def test_explode_table_excludes_start(self) -> None: assert all(locus in result_loci for locus in expected_loci) def test_explode_table_excludes_end(self) -> None: + """Test that the end position is excluded when exploding a table interval with includes_end=False.""" interval = hl.interval( start=hl.Locus("chr1", 100, "GRCh38"), end=hl.Locus("chr1", 105, "GRCh38"), @@ -378,6 +396,7 @@ def test_explode_table_excludes_end(self) -> None: assert all(locus in result_loci for locus in expected_loci) def test_explode_table_excludes_both(self) -> None: + """Test that both endpoints are excluded when exploding a table interval with includes_start and includes_end set to False.""" interval = hl.interval( start=hl.Locus("chr1", 100, "GRCh38"), end=hl.Locus("chr1", 105, "GRCh38"), @@ -400,6 +419,7 @@ def test_explode_table_excludes_both(self) -> None: assert all(locus in result_loci for locus in expected_loci) def test_explode_list_of_interval_expressions(self) -> None: + """Test exploding a list of non-overlapping IntervalExpressions returns all expected positions.""" intervals = [ hl.interval( hl.locus("chr1", 100, "GRCh38"), @@ -422,6 +442,7 @@ def test_explode_list_of_interval_expressions(self) -> None: assert set(positions) == {100, 101, 102, 200, 201} def test_explode_table_overlapping_intervals_deduplicates(self) -> None: + """Test that overlapping table intervals produce deduplicated loci when deduplicate=True.""" # chr1:100-105 and chr1:103-108, both inclusive → without dedup: 12 rows # with deduplicate=True (default): 9 distinct loci [100..108] intervals = [ @@ -459,6 +480,7 @@ def test_explode_table_overlapping_intervals_deduplicates(self) -> None: def test_explode_table_overlapping_intervals_keep_intervals_warns( self, caplog ) -> None: + """Test that a warning is emitted and duplicates remain when keep_intervals=True with overlapping intervals.""" # With keep_intervals=True and deduplicate=True, deduplication is skipped and # a warning is emitted; overlapping positions appear as duplicate rows. import logging @@ -501,7 +523,9 @@ def test_explode_table_overlapping_intervals_keep_intervals_warns( ) # 3 from GENE1 + 3 from GENE2, positions 101-102 duplicated def test_explode_list_overlapping_intervals_deduplicates(self) -> None: - # Two overlapping intervals; deduplicate=True (default) removes duplicate positions. + """Test that overlapping intervals in a list are deduplicated when deduplicate=True.""" + # Two overlapping intervals; deduplicate=True (default) removes duplicate + # positions. intervals = [ hl.interval( hl.locus("chr1", 100, "GRCh38"), @@ -525,6 +549,7 @@ def test_explode_list_overlapping_intervals_deduplicates(self) -> None: def test_explode_list_overlapping_intervals_no_deduplicate_warns( self, caplog ) -> None: + """Test that a warning is emitted and duplicates are present when deduplicate=False with overlapping list intervals.""" import logging intervals = [ From abe4557bebf056fcc70a26cf2b0da264e6d4e1af Mon Sep 17 00:00:00 2001 From: Ruchit10 Date: Mon, 27 Apr 2026 16:24:05 -0400 Subject: [PATCH 37/48] Fix pylint attempt 2 --- gnomad/utils/intervals.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 7f50cebc1..4c5416216 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -136,6 +136,7 @@ def explode_intervals_to_loci( Expand interval(s) to loci. If input is a Table, function will expand intervals to loci and key Table by loci. + If input is an IntervalExpression or a list of IntervalExpressions, function will return an ArrayExpression containing all loci within the input interval(s). .. warning:: @@ -143,13 +144,12 @@ def explode_intervals_to_loci( - When ``keep_intervals=True`` on a Table input, deduplication is not possible because duplicate rows with different interval annotations may exist; a warning is displayed instead. - Caution when using this function on very larg intervals (e.g. whole chromosomes), as it will create extremely large arrays, which may cause performance issues. - NOTE: Intervals that cross chromosomes is currently not supported. + Note that Intervals that cross chromosomes is currently not supported. :param intervals: Table, IntervalExpression, or list of IntervalExpressions. :param interval_field: Name of the interval field. Only required if input is a Hail Table. Default is None. :param keep_intervals: If True, keep the original intervals as a column in output. Only applies if input is a Hail Table. Default is False. - :param deduplicate: If True, remove duplicate loci produced by overlapping intervals. For Table input with ``keep_intervals=True``, deduplication is skipped with a warning. - For a list of IntervalExpressions, the returned ArrayExpression will have duplicate positions removed. Default is True. + :param deduplicate: If True, remove duplicate loci produced by overlapping intervals. For Table input with ``keep_intervals=True``, deduplication is skipped with a warning. For a list of IntervalExpressions, the returned ArrayExpression will have duplicate positions removed. Default is True. :return: If input is a Hail Table, returns exploded Table keyed by locus. If input is an IntervalExpression or list of IntervalExpressions, returns ArrayExpression containing loci within input interval(s). """ assert ( From 19a624ba855bf03208e1c4964e19a96924e78204 Mon Sep 17 00:00:00 2001 From: Ruchit10 Date: Mon, 27 Apr 2026 16:35:34 -0400 Subject: [PATCH 38/48] Fix test errors attempt 3 --- tests/utils/test_interval_utils.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/tests/utils/test_interval_utils.py b/tests/utils/test_interval_utils.py index 8ae735a52..4c01b0571 100644 --- a/tests/utils/test_interval_utils.py +++ b/tests/utils/test_interval_utils.py @@ -233,9 +233,7 @@ def test_explode_table_invalid_interval_field_raises_error( self, sample_interval_table: hl.Table ) -> None: """Test that an error is raised when interval_field is not present in the Table.""" - with pytest.raises( - AssertionError, match="`interval_field` must be an annotation" - ): + with pytest.raises(ValueError, match="`interval_field` must be an annotation"): explode_intervals_to_loci( sample_interval_table, interval_field="nonexistent_field", @@ -311,7 +309,11 @@ def test_explode_table_preserves_other_fields(self) -> None: def test_explode_invalid_input_type_raises_error(self) -> None: """Test that an AssertionError is raised when input is an unsupported type.""" with pytest.raises( - AssertionError, match="Input must be a Table or IntervalExpression" + AssertionError, + match=( + "Input must be a Table, IntervalExpression, or list of" + " IntervalExpressions" + ), ): explode_intervals_to_loci("invalid_input") @@ -519,8 +521,11 @@ def test_explode_table_overlapping_intervals_keep_intervals_warns( # Duplicate loci are present: positions 101 and 102 each appear twice. result_loci = [row.locus for row in result_ht.collect()] assert ( - len(result_loci) == 7 - ) # 3 from GENE1 + 3 from GENE2, positions 101-102 duplicated + len(result_loci) + == 6 + # 3 from GENE1 (100,101,102) + 3 from GENE2 (101,102,103), positions + # 101-102 duplicated + ) def test_explode_list_overlapping_intervals_deduplicates(self) -> None: """Test that overlapping intervals in a list are deduplicated when deduplicate=True.""" @@ -576,6 +581,6 @@ def test_explode_list_overlapping_intervals_no_deduplicate_warns( ) positions = hl.eval(result) # Positions 101 and 102 appear twice due to overlap. - assert len(positions) == 7 + assert len(positions) == 6 # [100,101,102] + [101,102,103] assert positions.count(101) == 2 assert positions.count(102) == 2 From d6645ce5f5909f328fae038e958026ba1af6ea21 Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Wed, 29 Apr 2026 16:14:48 -0400 Subject: [PATCH 39/48] Apply suggestions from code review Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 4 ++-- tests/utils/test_interval_utils.py | 10 +--------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 4c5416216..6c865d62b 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -142,9 +142,9 @@ def explode_intervals_to_loci( .. warning:: - Overlapping intervals will produce duplicate loci. Use ``deduplicate=True`` (the default) to remove them. - When ``keep_intervals=True`` on a Table input, deduplication is not possible because duplicate rows with different interval annotations may exist; a warning is displayed instead. - - Caution when using this function on very larg intervals (e.g. whole chromosomes), as it will create extremely large arrays, which may cause performance issues. + - Caution when using this function on very large intervals (e.g., whole chromosomes), as it will create extremely large arrays, which may cause performance issues. - Note that Intervals that cross chromosomes is currently not supported. + Note that Intervals that cross chromosomes are currently not supported. :param intervals: Table, IntervalExpression, or list of IntervalExpressions. :param interval_field: Name of the interval field. Only required if input is a Hail Table. Default is None. diff --git a/tests/utils/test_interval_utils.py b/tests/utils/test_interval_utils.py index 4c01b0571..d5d9eb05a 100644 --- a/tests/utils/test_interval_utils.py +++ b/tests/utils/test_interval_utils.py @@ -51,24 +51,16 @@ def test_explode_table_with_includes_start_and_end( self, sample_interval_table: hl.Table ) -> None: """Test exploding a table with intervals that include both start and end positions.""" - # Explode the intervals to loci. result_ht = explode_intervals_to_loci( sample_interval_table, interval_field="interval", keep_intervals=False ) - # Collect results. result = result_ht.collect() - - # Expected loci for the first interval (100-105, both inclusive). + expected_loci_1 = [hl.Locus("chr1", pos, "GRCh38") for pos in range(100, 106)] - # Expected loci for the second interval (200-203, start inclusive, end - # exclusive). expected_loci_2 = [hl.Locus("chr2", pos, "GRCh38") for pos in range(200, 203)] - # Get the loci from the result. result_loci = [row.locus for row in result] - - # Verify the result contains the expected loci. assert len(result_loci) == len(expected_loci_1) + len(expected_loci_2) assert all(locus in result_loci for locus in expected_loci_1) assert all(locus in result_loci for locus in expected_loci_2) From 9082e060432ba99bbd97f9d90d26423e74e7cd50 Mon Sep 17 00:00:00 2001 From: Ruchit10 Date: Mon, 4 May 2026 11:24:13 -0400 Subject: [PATCH 40/48] Addressing comments (round3) and adding more tests --- gnomad/utils/intervals.py | 95 +++++++----- tests/utils/test_interval_utils.py | 235 ++++++++++++++++++++--------- 2 files changed, 223 insertions(+), 107 deletions(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 6c865d62b..9f97dee5c 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -124,6 +124,30 @@ def _add_padding( return _add_padding(intervals) +def interval_to_pos_range( + interval_expr: hl.expr.IntervalExpression, +) -> hl.expr.ArrayExpression: + """ + Convert an IntervalExpression to an ArrayExpression of integer positions. + + Handles both inclusive and exclusive interval endpoints. + + :param interval_expr: IntervalExpression to convert. + :return: ArrayExpression of integer positions within the interval. + """ + start = hl.if_else( + interval_expr.includes_start, + interval_expr.start.position, + interval_expr.start.position + 1, + ) + end = hl.if_else( + interval_expr.includes_end, + interval_expr.end.position + 1, + interval_expr.end.position, + ) + return hl.range(start, end) + + def explode_intervals_to_loci( intervals: Union[ hl.Table, hl.expr.IntervalExpression, List[hl.expr.IntervalExpression] @@ -131,6 +155,7 @@ def explode_intervals_to_loci( interval_field: Optional[str] = None, keep_intervals: Optional[bool] = False, deduplicate: bool = True, + flatten: bool = True, ) -> Union[hl.Table, hl.expr.ArrayExpression]: """ Expand interval(s) to loci. @@ -149,8 +174,9 @@ def explode_intervals_to_loci( :param intervals: Table, IntervalExpression, or list of IntervalExpressions. :param interval_field: Name of the interval field. Only required if input is a Hail Table. Default is None. :param keep_intervals: If True, keep the original intervals as a column in output. Only applies if input is a Hail Table. Default is False. - :param deduplicate: If True, remove duplicate loci produced by overlapping intervals. For Table input with ``keep_intervals=True``, deduplication is skipped with a warning. For a list of IntervalExpressions, the returned ArrayExpression will have duplicate positions removed. Default is True. - :return: If input is a Hail Table, returns exploded Table keyed by locus. If input is an IntervalExpression or list of IntervalExpressions, returns ArrayExpression containing loci within input interval(s). + :param deduplicate: If True, remove duplicate loci produced by overlapping intervals. For Table input with ``keep_intervals=True``, deduplication is skipped with a warning. For a list of IntervalExpressions, the returned ArrayExpression will have duplicate loci removed. Has no effect when ``flatten=False``. Default is True. + :param flatten: If True, flatten the per-interval loci arrays into a single ArrayExpression. Only applies when input is a list of IntervalExpressions. Default is True. + :return: If input is a Hail Table, returns exploded Table keyed by locus. If input is an IntervalExpression or list of IntervalExpressions, returns ArrayExpression containing loci within input interval(s). If input is a list and ``flatten=False``, returns a nested ArrayExpression of per-interval loci arrays. """ assert ( isinstance(intervals, hl.Table) @@ -172,37 +198,36 @@ def explode_intervals_to_loci( raise ValueError( "`interval_field` must be an annotation present on input Table!" ) + if not isinstance(intervals[interval_field], hl.expr.IntervalExpression): + raise ValueError( + f"`interval_field` '{interval_field}' must refer to an interval field" + " in the input Table!" + ) if isinstance(intervals, list): logger.info( "Input is a list of IntervalExpressions, so function will return an" - " ArrayExpression of positions within all input intervals. To fully explode" - " intervals to loci, we recommend annotating your dataset with the returned" - " ArrayExpression, exploding the array, and converting the positions to" - " loci!" + " ArrayExpression of loci within all input intervals." ) - if not deduplicate: + if not deduplicate and flatten: logger.warning( "Overlapping intervals in the input list may produce duplicate loci in" " the returned ArrayExpression. Set `deduplicate=True` to remove them." ) - def _interval_to_range(interval_expr): - start = hl.if_else( - interval_expr.includes_start, - interval_expr.start.position, - interval_expr.start.position + 1, - ) - end = hl.if_else( - interval_expr.includes_end, - interval_expr.end.position + 1, - interval_expr.end.position, + loci_arrays = [ + interval_to_pos_range(i).map( + lambda pos, _i=i: hl.locus( + _i.start.contig, + pos, + reference_genome=_i.start.dtype.reference_genome, + ) ) - return hl.range(start, end) - - result = hl.array([_interval_to_range(i) for i in intervals]).flatmap( - lambda x: x - ) + for i in intervals + ] + if not flatten: + return hl.array(loci_arrays) + result = hl.flatten(hl.array(loci_arrays)) if deduplicate: result = hl.array(hl.set(result)) return result @@ -212,19 +237,9 @@ def _interval_to_range(interval_expr): if isinstance(intervals, hl.expr.IntervalExpression) else intervals[interval_field] ) - intervals_start_expr = hl.if_else( - intervals_expr.includes_start, - intervals_expr.start.position, - intervals_expr.start.position + 1, - ) - intervals_end_expr = hl.if_else( - intervals_expr.includes_end, - intervals_expr.end.position + 1, - intervals_expr.end.position, - ) if isinstance(intervals, hl.Table): intervals = intervals.annotate( - _pos=hl.range(intervals_start_expr, intervals_end_expr) + _pos=interval_to_pos_range(intervals_expr) ).explode("_pos") intervals = intervals.key_by( locus=hl.locus( @@ -254,9 +269,13 @@ def _interval_to_range(interval_expr): return intervals logger.info( - "Input is an IntervalExpression, so function will return ArrayExpression of" - " positions within input intervals. To fully explode intervals to loci, we" - " recommend annotating your dataset with the returned ArrayExpression," - " exploding the array, and converting the positions to loci!" + "Input is an IntervalExpression, so function will return an ArrayExpression of" + " loci within the input interval." + ) + return interval_to_pos_range(intervals_expr).map( + lambda pos: hl.locus( + intervals_expr.start.contig, + pos, + reference_genome=intervals_expr.start.dtype.reference_genome, + ) ) - return hl.range(intervals_start_expr, intervals_end_expr) diff --git a/tests/utils/test_interval_utils.py b/tests/utils/test_interval_utils.py index d5d9eb05a..bf872c12a 100644 --- a/tests/utils/test_interval_utils.py +++ b/tests/utils/test_interval_utils.py @@ -1,9 +1,11 @@ """Tests for the intervals utility module.""" +import logging + import hail as hl import pytest -from gnomad.utils.intervals import explode_intervals_to_loci +from gnomad.utils.intervals import explode_intervals_to_loci, interval_to_pos_range class TestExplodeIntervalsToLoci: @@ -13,15 +15,15 @@ class TestExplodeIntervalsToLoci: def sample_interval_table(self): """Fixture to create a sample Hail Table with intervals.""" intervals = [ - hl.Interval( - start=hl.Locus("chr1", 100, "GRCh38"), - end=hl.Locus("chr1", 105, "GRCh38"), + hl.interval( + start=hl.locus("chr1", 100, "GRCh38"), + end=hl.locus("chr1", 105, "GRCh38"), includes_start=True, includes_end=True, ), - hl.Interval( - start=hl.Locus("chr2", 200, "GRCh38"), - end=hl.Locus("chr2", 203, "GRCh38"), + hl.interval( + start=hl.locus("chr2", 200, "GRCh38"), + end=hl.locus("chr2", 203, "GRCh38"), includes_start=True, includes_end=False, ), @@ -56,9 +58,9 @@ def test_explode_table_with_includes_start_and_end( ) result = result_ht.collect() - - expected_loci_1 = [hl.Locus("chr1", pos, "GRCh38") for pos in range(100, 106)] - expected_loci_2 = [hl.Locus("chr2", pos, "GRCh38") for pos in range(200, 203)] + + expected_loci_1 = [hl.locus("chr1", pos, "GRCh38") for pos in range(100, 106)] + expected_loci_2 = [hl.locus("chr2", pos, "GRCh38") for pos in range(200, 203)] result_loci = [row.locus for row in result] assert len(result_loci) == len(expected_loci_1) + len(expected_loci_2) @@ -107,18 +109,18 @@ def test_explode_table_without_keep_intervals( def test_explode_interval_expression( self, sample_interval_expr: hl.expr.IntervalExpression ) -> None: - """Test exploding a single IntervalExpression returns the correct positions.""" + """Test exploding a single IntervalExpression returns the correct loci.""" # Explode the interval expression. result = explode_intervals_to_loci(sample_interval_expr) # Evaluate the result. - positions = hl.eval(result) + loci = hl.eval(result) - # Expected positions (100-105, both inclusive). - expected_positions = list(range(100, 106)) + # Expected loci (chr1:100-105, both inclusive). + expected_loci = [hl.locus("chr1", pos, "GRCh38") for pos in range(100, 106)] # Verify the result. - assert positions == expected_positions + assert loci == expected_loci def test_explode_interval_expression_excludes_start(self) -> None: """Test that the start position is excluded when includes_start=False.""" @@ -133,13 +135,13 @@ def test_explode_interval_expression_excludes_start(self) -> None: result = explode_intervals_to_loci(interval) # Evaluate the result. - positions = hl.eval(result) + loci = hl.eval(result) - # Expected positions (101-105, start excluded, end included). - expected_positions = list(range(101, 106)) + # Expected loci (chr1:101-105, start excluded, end included). + expected_loci = [hl.locus("chr1", pos, "GRCh38") for pos in range(101, 106)] # Verify the result. - assert positions == expected_positions + assert loci == expected_loci def test_explode_interval_expression_excludes_end(self) -> None: """Test that the end position is excluded when includes_end=False.""" @@ -154,13 +156,13 @@ def test_explode_interval_expression_excludes_end(self) -> None: result = explode_intervals_to_loci(interval) # Evaluate the result. - positions = hl.eval(result) + loci = hl.eval(result) - # Expected positions (100-104, start included, end excluded). - expected_positions = list(range(100, 105)) + # Expected loci (chr1:100-104, start included, end excluded). + expected_loci = [hl.locus("chr1", pos, "GRCh38") for pos in range(100, 105)] # Verify the result. - assert positions == expected_positions + assert loci == expected_loci def test_explode_interval_expression_excludes_both(self) -> None: """Test that both endpoints are excluded when includes_start and includes_end are False.""" @@ -175,19 +177,19 @@ def test_explode_interval_expression_excludes_both(self) -> None: result = explode_intervals_to_loci(interval) # Evaluate the result. - positions = hl.eval(result) + loci = hl.eval(result) - # Expected positions (101-104, both excluded). - expected_positions = list(range(101, 105)) + # Expected loci (chr1:101-104, both excluded). + expected_loci = [hl.locus("chr1", pos, "GRCh38") for pos in range(101, 105)] # Verify the result. - assert positions == expected_positions + assert loci == expected_loci def test_explode_table_single_position_interval(self) -> None: """Test exploding a table with a single-position interval that includes both endpoints.""" interval = hl.interval( - start=hl.Locus("chr1", 100, "GRCh38"), - end=hl.Locus("chr1", 100, "GRCh38"), + start=hl.locus("chr1", 100, "GRCh38"), + end=hl.locus("chr1", 100, "GRCh38"), includes_start=True, includes_end=True, ) @@ -210,7 +212,7 @@ def test_explode_table_single_position_interval(self) -> None: # Expected: single locus at position 100. assert len(result) == 1 - assert result[0].locus == hl.Locus("chr1", 100, "GRCh38") + assert result[0].locus == hl.locus("chr1", 100, "GRCh38") def test_explode_table_missing_interval_field_raises_error( self, sample_interval_table: hl.Table @@ -232,11 +234,22 @@ def test_explode_table_invalid_interval_field_raises_error( keep_intervals=False, ) + def test_explode_table_interval_field_wrong_type_raises_error( + self, sample_interval_table: hl.Table + ) -> None: + """Test that a ValueError is raised when interval_field refers to a non-interval field.""" + with pytest.raises(ValueError, match="must refer to an interval field"): + explode_intervals_to_loci( + sample_interval_table, + interval_field="gene", + keep_intervals=False, + ) + def test_explode_table_grch37(self) -> None: """Test exploding a table with GRCh37 reference genome intervals.""" interval = hl.interval( - start=hl.Locus("1", 1000, "GRCh37"), - end=hl.Locus("1", 1003, "GRCh37"), + start=hl.locus("1", 1000, "GRCh37"), + end=hl.locus("1", 1003, "GRCh37"), includes_start=True, includes_end=True, ) @@ -258,7 +271,7 @@ def test_explode_table_grch37(self) -> None: result = result_ht.collect() # Expected loci (1000-1003, both inclusive). - expected_loci = [hl.Locus("1", pos, "GRCh37") for pos in range(1000, 1004)] + expected_loci = [hl.locus("1", pos, "GRCh37") for pos in range(1000, 1004)] # Get the loci from the result. result_loci = [row.locus for row in result] @@ -270,8 +283,8 @@ def test_explode_table_grch37(self) -> None: def test_explode_table_preserves_other_fields(self) -> None: """Test that non-interval fields are preserved after exploding.""" interval = hl.interval( - start=hl.Locus("chr1", 100, "GRCh38"), - end=hl.Locus("chr1", 102, "GRCh38"), + start=hl.locus("chr1", 100, "GRCh38"), + end=hl.locus("chr1", 102, "GRCh38"), includes_start=True, includes_end=True, ) @@ -319,15 +332,15 @@ def test_explode_interval_expression_single_position_excludes_both(self) -> None ) result = explode_intervals_to_loci(interval) - positions = hl.eval(result) + loci = hl.eval(result) - assert positions == [] + assert loci == [] def test_explode_table_single_position_interval_excludes_both(self) -> None: """Test that a table with a single-position interval excluding both endpoints returns 0 rows.""" interval = hl.interval( - start=hl.Locus("chr1", 100, "GRCh38"), - end=hl.Locus("chr1", 100, "GRCh38"), + start=hl.locus("chr1", 100, "GRCh38"), + end=hl.locus("chr1", 100, "GRCh38"), includes_start=False, includes_end=False, ) @@ -346,8 +359,8 @@ def test_explode_table_single_position_interval_excludes_both(self) -> None: def test_explode_table_excludes_start(self) -> None: """Test that the start position is excluded when exploding a table interval with includes_start=False.""" interval = hl.interval( - start=hl.Locus("chr1", 100, "GRCh38"), - end=hl.Locus("chr1", 105, "GRCh38"), + start=hl.locus("chr1", 100, "GRCh38"), + end=hl.locus("chr1", 105, "GRCh38"), includes_start=False, includes_end=True, ) @@ -362,15 +375,15 @@ def test_explode_table_excludes_start(self) -> None: ) result_loci = [row.locus for row in result_ht.collect()] - expected_loci = [hl.Locus("chr1", pos, "GRCh38") for pos in range(101, 106)] + expected_loci = [hl.locus("chr1", pos, "GRCh38") for pos in range(101, 106)] assert len(result_loci) == len(expected_loci) assert all(locus in result_loci for locus in expected_loci) def test_explode_table_excludes_end(self) -> None: """Test that the end position is excluded when exploding a table interval with includes_end=False.""" interval = hl.interval( - start=hl.Locus("chr1", 100, "GRCh38"), - end=hl.Locus("chr1", 105, "GRCh38"), + start=hl.locus("chr1", 100, "GRCh38"), + end=hl.locus("chr1", 105, "GRCh38"), includes_start=True, includes_end=False, ) @@ -385,15 +398,15 @@ def test_explode_table_excludes_end(self) -> None: ) result_loci = [row.locus for row in result_ht.collect()] - expected_loci = [hl.Locus("chr1", pos, "GRCh38") for pos in range(100, 105)] + expected_loci = [hl.locus("chr1", pos, "GRCh38") for pos in range(100, 105)] assert len(result_loci) == len(expected_loci) assert all(locus in result_loci for locus in expected_loci) def test_explode_table_excludes_both(self) -> None: """Test that both endpoints are excluded when exploding a table interval with includes_start and includes_end set to False.""" interval = hl.interval( - start=hl.Locus("chr1", 100, "GRCh38"), - end=hl.Locus("chr1", 105, "GRCh38"), + start=hl.locus("chr1", 100, "GRCh38"), + end=hl.locus("chr1", 105, "GRCh38"), includes_start=False, includes_end=False, ) @@ -408,12 +421,12 @@ def test_explode_table_excludes_both(self) -> None: ) result_loci = [row.locus for row in result_ht.collect()] - expected_loci = [hl.Locus("chr1", pos, "GRCh38") for pos in range(101, 105)] + expected_loci = [hl.locus("chr1", pos, "GRCh38") for pos in range(101, 105)] assert len(result_loci) == len(expected_loci) assert all(locus in result_loci for locus in expected_loci) def test_explode_list_of_interval_expressions(self) -> None: - """Test exploding a list of non-overlapping IntervalExpressions returns all expected positions.""" + """Test exploding a list of non-overlapping IntervalExpressions returns all expected loci.""" intervals = [ hl.interval( hl.locus("chr1", 100, "GRCh38"), @@ -430,10 +443,16 @@ def test_explode_list_of_interval_expressions(self) -> None: ] result = explode_intervals_to_loci(intervals) - positions = hl.eval(result) + loci = hl.eval(result) - # deduplicate=True by default; set comparison since order is not guaranteed. - assert set(positions) == {100, 101, 102, 200, 201} + expected_loci = { + hl.locus("chr1", 100, "GRCh38"), + hl.locus("chr1", 101, "GRCh38"), + hl.locus("chr1", 102, "GRCh38"), + hl.locus("chr2", 200, "GRCh38"), + hl.locus("chr2", 201, "GRCh38"), + } + assert set(loci) == expected_loci def test_explode_table_overlapping_intervals_deduplicates(self) -> None: """Test that overlapping table intervals produce deduplicated loci when deduplicate=True.""" @@ -441,14 +460,14 @@ def test_explode_table_overlapping_intervals_deduplicates(self) -> None: # with deduplicate=True (default): 9 distinct loci [100..108] intervals = [ hl.interval( - start=hl.Locus("chr1", 100, "GRCh38"), - end=hl.Locus("chr1", 105, "GRCh38"), + start=hl.locus("chr1", 100, "GRCh38"), + end=hl.locus("chr1", 105, "GRCh38"), includes_start=True, includes_end=True, ), hl.interval( - start=hl.Locus("chr1", 103, "GRCh38"), - end=hl.Locus("chr1", 108, "GRCh38"), + start=hl.locus("chr1", 103, "GRCh38"), + end=hl.locus("chr1", 108, "GRCh38"), includes_start=True, includes_end=True, ), @@ -467,28 +486,60 @@ def test_explode_table_overlapping_intervals_deduplicates(self) -> None: ) result_loci = [row.locus for row in result_ht.collect()] - expected_loci = [hl.Locus("chr1", pos, "GRCh38") for pos in range(100, 109)] + expected_loci = [hl.locus("chr1", pos, "GRCh38") for pos in range(100, 109)] assert len(result_loci) == len(expected_loci) assert all(locus in result_loci for locus in expected_loci) + def test_explode_table_overlapping_intervals_no_deduplicate(self) -> None: + """Test that overlapping table intervals produce duplicate loci when deduplicate=False.""" + # chr1:100-105 and chr1:103-108, both inclusive. + # Without deduplication: 6 rows from GENE1 + 6 rows from GENE2 = 12 rows total. + intervals = [ + hl.interval( + start=hl.locus("chr1", 100, "GRCh38"), + end=hl.locus("chr1", 105, "GRCh38"), + includes_start=True, + includes_end=True, + ), + hl.interval( + start=hl.locus("chr1", 103, "GRCh38"), + end=hl.locus("chr1", 108, "GRCh38"), + includes_start=True, + includes_end=True, + ), + ] + + ht = hl.Table.parallelize( + [ + {"interval": intervals[0], "gene": "GENE1"}, + {"interval": intervals[1], "gene": "GENE2"}, + ], + hl.tstruct(interval=hl.tinterval(hl.tlocus("GRCh38")), gene=hl.tstr), + ) + + result_ht = explode_intervals_to_loci( + ht, interval_field="interval", keep_intervals=False, deduplicate=False + ) + # Overlapping loci (chr1:103-105) appear twice. + assert result_ht.count() == 12 + def test_explode_table_overlapping_intervals_keep_intervals_warns( self, caplog ) -> None: """Test that a warning is emitted and duplicates remain when keep_intervals=True with overlapping intervals.""" # With keep_intervals=True and deduplicate=True, deduplication is skipped and # a warning is emitted; overlapping positions appear as duplicate rows. - import logging intervals = [ hl.interval( - start=hl.Locus("chr1", 100, "GRCh38"), - end=hl.Locus("chr1", 102, "GRCh38"), + start=hl.locus("chr1", 100, "GRCh38"), + end=hl.locus("chr1", 102, "GRCh38"), includes_start=True, includes_end=True, ), hl.interval( - start=hl.Locus("chr1", 101, "GRCh38"), - end=hl.Locus("chr1", 103, "GRCh38"), + start=hl.locus("chr1", 101, "GRCh38"), + end=hl.locus("chr1", 103, "GRCh38"), includes_start=True, includes_end=True, ), @@ -539,9 +590,10 @@ def test_explode_list_overlapping_intervals_deduplicates(self) -> None: ] result = explode_intervals_to_loci(intervals, deduplicate=True) - positions = hl.eval(result) + loci = hl.eval(result) - assert set(positions) == set(range(100, 109)) + expected_loci = {hl.locus("chr1", pos, "GRCh38") for pos in range(100, 109)} + assert set(loci) == expected_loci def test_explode_list_overlapping_intervals_no_deduplicate_warns( self, caplog @@ -571,8 +623,53 @@ def test_explode_list_overlapping_intervals_no_deduplicate_warns( "Overlapping intervals in the input list may produce duplicate loci" in caplog.text ) - positions = hl.eval(result) - # Positions 101 and 102 appear twice due to overlap. - assert len(positions) == 6 # [100,101,102] + [101,102,103] - assert positions.count(101) == 2 - assert positions.count(102) == 2 + loci = hl.eval(result) + # Loci at chr1:101 and chr1:102 appear twice due to overlap. + assert len(loci) == 6 + assert loci.count(hl.locus("chr1", 101, "GRCh38")) == 2 + assert loci.count(hl.locus("chr1", 102, "GRCh38")) == 2 + + def test_explode_empty_list_raises_type_error(self) -> None: + """Test that passing an empty Python list raises a TypeError. + + Hail cannot infer the element type of an empty Python list, so + ``hl.array([])`` (called internally) raises a TypeError. This is distinct + from passing a Hail-typed empty array expression, which would instead + fail the input-type assertion because ``ArrayExpression`` is not a + supported input type. + """ + with pytest.raises(TypeError): + explode_intervals_to_loci([]) + + def test_explode_list_cross_chromosome_same_position_deduplicates_correctly( + self, + ) -> None: + """Test that loci on different chromosomes with the same integer position are not conflated. + + chr1:100-102 and chr2:100-102 share the same position numbers but are + distinct loci. Locus-based deduplication must preserve all 6 loci; a + position-only deduplication strategy would incorrectly collapse them to 3. + """ + intervals = [ + hl.interval( + hl.locus("chr1", 100, "GRCh38"), + hl.locus("chr1", 102, "GRCh38"), + includes_start=True, + includes_end=True, + ), + hl.interval( + hl.locus("chr2", 100, "GRCh38"), + hl.locus("chr2", 102, "GRCh38"), + includes_start=True, + includes_end=True, + ), + ] + + result = explode_intervals_to_loci(intervals, deduplicate=True) + loci = hl.eval(result) + + expected_loci = [hl.locus("chr1", pos, "GRCh38") for pos in range(100, 103)] + [ + hl.locus("chr2", pos, "GRCh38") for pos in range(100, 103) + ] + assert len(loci) == 6 + assert set(loci) == set(expected_loci) From 435c1d92ef0b57c13f7360f138b8c7563a7e7cdb Mon Sep 17 00:00:00 2001 From: Ruchit10 Date: Mon, 4 May 2026 12:36:33 -0400 Subject: [PATCH 41/48] Fix pylint issues --- tests/utils/test_interval_utils.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/utils/test_interval_utils.py b/tests/utils/test_interval_utils.py index bf872c12a..61d1f0e9b 100644 --- a/tests/utils/test_interval_utils.py +++ b/tests/utils/test_interval_utils.py @@ -5,7 +5,7 @@ import hail as hl import pytest -from gnomad.utils.intervals import explode_intervals_to_loci, interval_to_pos_range +from gnomad.utils.intervals import explode_intervals_to_loci class TestExplodeIntervalsToLoci: @@ -599,8 +599,6 @@ def test_explode_list_overlapping_intervals_no_deduplicate_warns( self, caplog ) -> None: """Test that a warning is emitted and duplicates are present when deduplicate=False with overlapping list intervals.""" - import logging - intervals = [ hl.interval( hl.locus("chr1", 100, "GRCh38"), From 4a410a302f7c249ca11256ad20fa5e30c4c9eda9 Mon Sep 17 00:00:00 2001 From: Ruchit10 Date: Tue, 5 May 2026 11:47:48 -0400 Subject: [PATCH 42/48] fix testing errors --- gnomad/utils/intervals.py | 14 +++++----- tests/utils/test_interval_utils.py | 44 +++++++++++++++--------------- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 9f97dee5c..8a3dbc00b 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -215,16 +215,16 @@ def explode_intervals_to_loci( " the returned ArrayExpression. Set `deduplicate=True` to remove them." ) - loci_arrays = [ - interval_to_pos_range(i).map( - lambda pos, _i=i: hl.locus( - _i.start.contig, + def _make_loci_array(interval_expr): + return interval_to_pos_range(interval_expr).map( + lambda pos: hl.locus( + interval_expr.start.contig, pos, - reference_genome=_i.start.dtype.reference_genome, + reference_genome=interval_expr.start.dtype.reference_genome, ) ) - for i in intervals - ] + + loci_arrays = [_make_loci_array(i) for i in intervals] if not flatten: return hl.array(loci_arrays) result = hl.flatten(hl.array(loci_arrays)) diff --git a/tests/utils/test_interval_utils.py b/tests/utils/test_interval_utils.py index 61d1f0e9b..1fc011c9c 100644 --- a/tests/utils/test_interval_utils.py +++ b/tests/utils/test_interval_utils.py @@ -59,8 +59,8 @@ def test_explode_table_with_includes_start_and_end( result = result_ht.collect() - expected_loci_1 = [hl.locus("chr1", pos, "GRCh38") for pos in range(100, 106)] - expected_loci_2 = [hl.locus("chr2", pos, "GRCh38") for pos in range(200, 203)] + expected_loci_1 = [hl.Locus("chr1", pos, "GRCh38") for pos in range(100, 106)] + expected_loci_2 = [hl.Locus("chr2", pos, "GRCh38") for pos in range(200, 203)] result_loci = [row.locus for row in result] assert len(result_loci) == len(expected_loci_1) + len(expected_loci_2) @@ -117,7 +117,7 @@ def test_explode_interval_expression( loci = hl.eval(result) # Expected loci (chr1:100-105, both inclusive). - expected_loci = [hl.locus("chr1", pos, "GRCh38") for pos in range(100, 106)] + expected_loci = [hl.Locus("chr1", pos, "GRCh38") for pos in range(100, 106)] # Verify the result. assert loci == expected_loci @@ -138,7 +138,7 @@ def test_explode_interval_expression_excludes_start(self) -> None: loci = hl.eval(result) # Expected loci (chr1:101-105, start excluded, end included). - expected_loci = [hl.locus("chr1", pos, "GRCh38") for pos in range(101, 106)] + expected_loci = [hl.Locus("chr1", pos, "GRCh38") for pos in range(101, 106)] # Verify the result. assert loci == expected_loci @@ -159,7 +159,7 @@ def test_explode_interval_expression_excludes_end(self) -> None: loci = hl.eval(result) # Expected loci (chr1:100-104, start included, end excluded). - expected_loci = [hl.locus("chr1", pos, "GRCh38") for pos in range(100, 105)] + expected_loci = [hl.Locus("chr1", pos, "GRCh38") for pos in range(100, 105)] # Verify the result. assert loci == expected_loci @@ -180,7 +180,7 @@ def test_explode_interval_expression_excludes_both(self) -> None: loci = hl.eval(result) # Expected loci (chr1:101-104, both excluded). - expected_loci = [hl.locus("chr1", pos, "GRCh38") for pos in range(101, 105)] + expected_loci = [hl.Locus("chr1", pos, "GRCh38") for pos in range(101, 105)] # Verify the result. assert loci == expected_loci @@ -212,7 +212,7 @@ def test_explode_table_single_position_interval(self) -> None: # Expected: single locus at position 100. assert len(result) == 1 - assert result[0].locus == hl.locus("chr1", 100, "GRCh38") + assert result[0].locus == hl.Locus("chr1", 100, "GRCh38") def test_explode_table_missing_interval_field_raises_error( self, sample_interval_table: hl.Table @@ -271,7 +271,7 @@ def test_explode_table_grch37(self) -> None: result = result_ht.collect() # Expected loci (1000-1003, both inclusive). - expected_loci = [hl.locus("1", pos, "GRCh37") for pos in range(1000, 1004)] + expected_loci = [hl.Locus("1", pos, "GRCh37") for pos in range(1000, 1004)] # Get the loci from the result. result_loci = [row.locus for row in result] @@ -375,7 +375,7 @@ def test_explode_table_excludes_start(self) -> None: ) result_loci = [row.locus for row in result_ht.collect()] - expected_loci = [hl.locus("chr1", pos, "GRCh38") for pos in range(101, 106)] + expected_loci = [hl.Locus("chr1", pos, "GRCh38") for pos in range(101, 106)] assert len(result_loci) == len(expected_loci) assert all(locus in result_loci for locus in expected_loci) @@ -398,7 +398,7 @@ def test_explode_table_excludes_end(self) -> None: ) result_loci = [row.locus for row in result_ht.collect()] - expected_loci = [hl.locus("chr1", pos, "GRCh38") for pos in range(100, 105)] + expected_loci = [hl.Locus("chr1", pos, "GRCh38") for pos in range(100, 105)] assert len(result_loci) == len(expected_loci) assert all(locus in result_loci for locus in expected_loci) @@ -421,7 +421,7 @@ def test_explode_table_excludes_both(self) -> None: ) result_loci = [row.locus for row in result_ht.collect()] - expected_loci = [hl.locus("chr1", pos, "GRCh38") for pos in range(101, 105)] + expected_loci = [hl.Locus("chr1", pos, "GRCh38") for pos in range(101, 105)] assert len(result_loci) == len(expected_loci) assert all(locus in result_loci for locus in expected_loci) @@ -446,11 +446,11 @@ def test_explode_list_of_interval_expressions(self) -> None: loci = hl.eval(result) expected_loci = { - hl.locus("chr1", 100, "GRCh38"), - hl.locus("chr1", 101, "GRCh38"), - hl.locus("chr1", 102, "GRCh38"), - hl.locus("chr2", 200, "GRCh38"), - hl.locus("chr2", 201, "GRCh38"), + hl.Locus("chr1", 100, "GRCh38"), + hl.Locus("chr1", 101, "GRCh38"), + hl.Locus("chr1", 102, "GRCh38"), + hl.Locus("chr2", 200, "GRCh38"), + hl.Locus("chr2", 201, "GRCh38"), } assert set(loci) == expected_loci @@ -486,7 +486,7 @@ def test_explode_table_overlapping_intervals_deduplicates(self) -> None: ) result_loci = [row.locus for row in result_ht.collect()] - expected_loci = [hl.locus("chr1", pos, "GRCh38") for pos in range(100, 109)] + expected_loci = [hl.Locus("chr1", pos, "GRCh38") for pos in range(100, 109)] assert len(result_loci) == len(expected_loci) assert all(locus in result_loci for locus in expected_loci) @@ -592,7 +592,7 @@ def test_explode_list_overlapping_intervals_deduplicates(self) -> None: result = explode_intervals_to_loci(intervals, deduplicate=True) loci = hl.eval(result) - expected_loci = {hl.locus("chr1", pos, "GRCh38") for pos in range(100, 109)} + expected_loci = {hl.Locus("chr1", pos, "GRCh38") for pos in range(100, 109)} assert set(loci) == expected_loci def test_explode_list_overlapping_intervals_no_deduplicate_warns( @@ -624,8 +624,8 @@ def test_explode_list_overlapping_intervals_no_deduplicate_warns( loci = hl.eval(result) # Loci at chr1:101 and chr1:102 appear twice due to overlap. assert len(loci) == 6 - assert loci.count(hl.locus("chr1", 101, "GRCh38")) == 2 - assert loci.count(hl.locus("chr1", 102, "GRCh38")) == 2 + assert loci.count(hl.Locus("chr1", 101, "GRCh38")) == 2 + assert loci.count(hl.Locus("chr1", 102, "GRCh38")) == 2 def test_explode_empty_list_raises_type_error(self) -> None: """Test that passing an empty Python list raises a TypeError. @@ -666,8 +666,8 @@ def test_explode_list_cross_chromosome_same_position_deduplicates_correctly( result = explode_intervals_to_loci(intervals, deduplicate=True) loci = hl.eval(result) - expected_loci = [hl.locus("chr1", pos, "GRCh38") for pos in range(100, 103)] + [ - hl.locus("chr2", pos, "GRCh38") for pos in range(100, 103) + expected_loci = [hl.Locus("chr1", pos, "GRCh38") for pos in range(100, 103)] + [ + hl.Locus("chr2", pos, "GRCh38") for pos in range(100, 103) ] assert len(loci) == 6 assert set(loci) == set(expected_loci) From e13373c11af208f9b8026d8469351a830fdcca8d Mon Sep 17 00:00:00 2001 From: Ruchit10 Date: Fri, 15 May 2026 12:26:10 -0400 Subject: [PATCH 43/48] Change error messaging for interval expression check --- gnomad/utils/intervals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 8a3dbc00b..e050cb962 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -200,7 +200,7 @@ def explode_intervals_to_loci( ) if not isinstance(intervals[interval_field], hl.expr.IntervalExpression): raise ValueError( - f"`interval_field` '{interval_field}' must refer to an interval field" + f"`interval_field` '{interval_field}' must be a hail interval expression" " in the input Table!" ) From 0fe4259d7bb350c9aed625891343ac5d4730accc Mon Sep 17 00:00:00 2001 From: Ruchit10 Date: Fri, 15 May 2026 12:27:36 -0400 Subject: [PATCH 44/48] Black formatting --- gnomad/utils/intervals.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index e050cb962..3f540fdb4 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -200,8 +200,8 @@ def explode_intervals_to_loci( ) if not isinstance(intervals[interval_field], hl.expr.IntervalExpression): raise ValueError( - f"`interval_field` '{interval_field}' must be a hail interval expression" - " in the input Table!" + f"`interval_field` '{interval_field}' must be a hail interval" + " expression in the input Table!" ) if isinstance(intervals, list): From 658fbd2f99676afd2ab69a89c35c437a6a99fb54 Mon Sep 17 00:00:00 2001 From: Ruchit10 Date: Fri, 15 May 2026 12:52:51 -0400 Subject: [PATCH 45/48] Change test assertion to match error message changes --- tests/utils/test_interval_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/utils/test_interval_utils.py b/tests/utils/test_interval_utils.py index 1fc011c9c..182473f16 100644 --- a/tests/utils/test_interval_utils.py +++ b/tests/utils/test_interval_utils.py @@ -238,7 +238,7 @@ def test_explode_table_interval_field_wrong_type_raises_error( self, sample_interval_table: hl.Table ) -> None: """Test that a ValueError is raised when interval_field refers to a non-interval field.""" - with pytest.raises(ValueError, match="must refer to an interval field"): + with pytest.raises(ValueError, match="must be a hail interval"): explode_intervals_to_loci( sample_interval_table, interval_field="gene", From 8be66f61c05a2cdc17dc7f683117fe669dd74c87 Mon Sep 17 00:00:00 2001 From: Ruchit Panchal Date: Fri, 15 May 2026 14:35:41 -0400 Subject: [PATCH 46/48] Apply suggestions from code review Co-authored-by: Katherine Chao --- gnomad/utils/intervals.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index 3f540fdb4..af8f5bcac 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -169,7 +169,7 @@ def explode_intervals_to_loci( - When ``keep_intervals=True`` on a Table input, deduplication is not possible because duplicate rows with different interval annotations may exist; a warning is displayed instead. - Caution when using this function on very large intervals (e.g., whole chromosomes), as it will create extremely large arrays, which may cause performance issues. - Note that Intervals that cross chromosomes are currently not supported. + Note that intervals that cross chromosomes are currently not supported. :param intervals: Table, IntervalExpression, or list of IntervalExpressions. :param interval_field: Name of the interval field. Only required if input is a Hail Table. Default is None. @@ -200,8 +200,8 @@ def explode_intervals_to_loci( ) if not isinstance(intervals[interval_field], hl.expr.IntervalExpression): raise ValueError( - f"`interval_field` '{interval_field}' must be a hail interval" - " expression in the input Table!" + f"`interval_field` '{interval_field}' must be an IntervalExpression" + " in the input Table!" ) if isinstance(intervals, list): From e9ef49b1cddec36c7a278baef9a7950c0374fca8 Mon Sep 17 00:00:00 2001 From: Ruchit10 Date: Fri, 15 May 2026 19:05:41 -0400 Subject: [PATCH 47/48] Address comments (round4) and adjust tests accordingly --- gnomad/utils/intervals.py | 131 +++++++++++++------------ tests/utils/test_interval_utils.py | 147 +++++++++++++++-------------- 2 files changed, 149 insertions(+), 129 deletions(-) diff --git a/gnomad/utils/intervals.py b/gnomad/utils/intervals.py index af8f5bcac..203fe6d33 100644 --- a/gnomad/utils/intervals.py +++ b/gnomad/utils/intervals.py @@ -5,8 +5,6 @@ import hail as hl -from gnomad.utils.reference_genome import get_reference_genome - logging.basicConfig( format="%(asctime)s (%(name)s %(lineno)s): %(message)s", datefmt="%m/%d/%Y %I:%M:%S %p", @@ -149,9 +147,7 @@ def interval_to_pos_range( def explode_intervals_to_loci( - intervals: Union[ - hl.Table, hl.expr.IntervalExpression, List[hl.expr.IntervalExpression] - ], + intervals: Union[hl.Table, hl.expr.IntervalExpression, hl.expr.ArrayExpression], interval_field: Optional[str] = None, keep_intervals: Optional[bool] = False, deduplicate: bool = True, @@ -162,30 +158,50 @@ def explode_intervals_to_loci( If input is a Table, function will expand intervals to loci and key Table by loci. - If input is an IntervalExpression or a list of IntervalExpressions, function will return an ArrayExpression containing all loci within the input interval(s). + If input is an IntervalExpression or an ArrayExpression of IntervalExpressions, + function will return an ArrayExpression containing all loci within the input + interval(s). .. warning:: - - Overlapping intervals will produce duplicate loci. Use ``deduplicate=True`` (the default) to remove them. - - When ``keep_intervals=True`` on a Table input, deduplication is not possible because duplicate rows with different interval annotations may exist; a warning is displayed instead. - - Caution when using this function on very large intervals (e.g., whole chromosomes), as it will create extremely large arrays, which may cause performance issues. + - Overlapping intervals will produce duplicate loci. Use ``deduplicate=True`` + (default) to remove them. + - When ``keep_intervals=True`` on a Table input, deduplication is not possible + because duplicate rows with different interval annotations may exist; a warning + is displayed instead. + - Caution when using this function on very large intervals (e.g., whole + chromosomes), as it will create extremely large arrays, which may cause + performance issues. Note that intervals that cross chromosomes are currently not supported. - :param intervals: Table, IntervalExpression, or list of IntervalExpressions. - :param interval_field: Name of the interval field. Only required if input is a Hail Table. Default is None. - :param keep_intervals: If True, keep the original intervals as a column in output. Only applies if input is a Hail Table. Default is False. - :param deduplicate: If True, remove duplicate loci produced by overlapping intervals. For Table input with ``keep_intervals=True``, deduplication is skipped with a warning. For a list of IntervalExpressions, the returned ArrayExpression will have duplicate loci removed. Has no effect when ``flatten=False``. Default is True. - :param flatten: If True, flatten the per-interval loci arrays into a single ArrayExpression. Only applies when input is a list of IntervalExpressions. Default is True. - :return: If input is a Hail Table, returns exploded Table keyed by locus. If input is an IntervalExpression or list of IntervalExpressions, returns ArrayExpression containing loci within input interval(s). If input is a list and ``flatten=False``, returns a nested ArrayExpression of per-interval loci arrays. + :param intervals: Table, IntervalExpression, or ArrayExpression of + IntervalExpressions. + :param interval_field: Name of the interval field. Only required if input is a Hail + Table. Default is None. + :param keep_intervals: If True, keep the original intervals as a column in output. + Only applies if input is a Hail Table. Default is False. + :param deduplicate: If True, remove duplicate loci produced by overlapping + intervals. For Table input with ``keep_intervals=True``, deduplication is + skipped with a warning. For an ArrayExpression of IntervalExpressions, the + returned ArrayExpression will have duplicate loci removed. Has no effect when + ``flatten=False``. Default is True. + :param flatten: If True, flatten the per-interval loci arrays into a single + ArrayExpression. Only applies when input is an ArrayExpression of + IntervalExpressions. Default is True. + :return: If input is a Hail Table, returns exploded Table keyed by locus. If input + is an IntervalExpression or ArrayExpression of IntervalExpressions, returns + ArrayExpression containing loci within input interval(s). If input is an + ArrayExpression and ``flatten=False``, returns a nested ArrayExpression of + per-interval loci arrays. """ assert ( isinstance(intervals, hl.Table) or isinstance(intervals, hl.expr.IntervalExpression) - or ( - isinstance(intervals, list) - and all(isinstance(i, hl.expr.IntervalExpression) for i in intervals) - ) - ), "Input must be a Table, IntervalExpression, or list of IntervalExpressions!" + or isinstance(intervals, hl.expr.ArrayExpression) + ), ( + "Input must be a Table, IntervalExpression, or ArrayExpression of" + " IntervalExpressions!" + ) if isinstance(intervals, hl.Table) and ( not interval_field or keep_intervals is None @@ -204,52 +220,49 @@ def explode_intervals_to_loci( " in the input Table!" ) - if isinstance(intervals, list): + def _make_loci_array(interval_expr): + return interval_to_pos_range(interval_expr).map( + lambda pos: hl.locus( + interval_expr.start.contig, + pos, + reference_genome=interval_expr.start.dtype.reference_genome, + ) + ) + + if isinstance(intervals, hl.expr.ArrayExpression): logger.info( - "Input is a list of IntervalExpressions, so function will return an" - " ArrayExpression of loci within all input intervals." + "Input is an ArrayExpression of IntervalExpressions, so function will" + " return an ArrayExpression of loci within all input intervals." ) if not deduplicate and flatten: logger.warning( - "Overlapping intervals in the input list may produce duplicate loci in" - " the returned ArrayExpression. Set `deduplicate=True` to remove them." - ) - - def _make_loci_array(interval_expr): - return interval_to_pos_range(interval_expr).map( - lambda pos: hl.locus( - interval_expr.start.contig, - pos, - reference_genome=interval_expr.start.dtype.reference_genome, - ) + "Overlapping intervals in the input array may produce duplicate loci" + " in the returned ArrayExpression. Set `deduplicate=True` to remove" + " them." ) - loci_arrays = [_make_loci_array(i) for i in intervals] - if not flatten: - return hl.array(loci_arrays) - result = hl.flatten(hl.array(loci_arrays)) - if deduplicate: - result = hl.array(hl.set(result)) - return result + loci_arrays = intervals.map(lambda i: _make_loci_array(i)) + if flatten: + result = hl.flatten(loci_arrays) + if deduplicate: + result = hl.array(hl.set(result)) + result = hl.sorted(result) + return result + else: + return loci_arrays intervals_expr = ( intervals if isinstance(intervals, hl.expr.IntervalExpression) else intervals[interval_field] ) + loci_array = _make_loci_array(intervals_expr) + if isinstance(intervals, hl.Table): - intervals = intervals.annotate( - _pos=interval_to_pos_range(intervals_expr) - ).explode("_pos") - intervals = intervals.key_by( - locus=hl.locus( - intervals[interval_field].start.contig, - intervals._pos, - reference_genome=get_reference_genome(intervals[interval_field]), - ) - ) + intervals = intervals.annotate(_loci=loci_array).explode("_loci") + intervals = intervals.key_by(locus=intervals._loci) - fields_to_drop = ["_pos"] + fields_to_drop = ["_loci"] if not keep_intervals: fields_to_drop.append(interval_field) @@ -265,6 +278,12 @@ def _make_loci_array(interval_expr): ) else: intervals = intervals.distinct() + logger.warning( + "The `distinct()` call will arbitrarily select one row for each" + " duplicated locus. If the input table has annotations such as gene" + " or transcript ID, the values retained for duplicated loci are not" + " deterministic." + ) return intervals @@ -272,10 +291,4 @@ def _make_loci_array(interval_expr): "Input is an IntervalExpression, so function will return an ArrayExpression of" " loci within the input interval." ) - return interval_to_pos_range(intervals_expr).map( - lambda pos: hl.locus( - intervals_expr.start.contig, - pos, - reference_genome=intervals_expr.start.dtype.reference_genome, - ) - ) + return loci_array diff --git a/tests/utils/test_interval_utils.py b/tests/utils/test_interval_utils.py index 182473f16..10f51c581 100644 --- a/tests/utils/test_interval_utils.py +++ b/tests/utils/test_interval_utils.py @@ -316,7 +316,7 @@ def test_explode_invalid_input_type_raises_error(self) -> None: with pytest.raises( AssertionError, match=( - "Input must be a Table, IntervalExpression, or list of" + "Input must be a Table, IntervalExpression, or ArrayExpression of" " IntervalExpressions" ), ): @@ -426,21 +426,23 @@ def test_explode_table_excludes_both(self) -> None: assert all(locus in result_loci for locus in expected_loci) def test_explode_list_of_interval_expressions(self) -> None: - """Test exploding a list of non-overlapping IntervalExpressions returns all expected loci.""" - intervals = [ - hl.interval( - hl.locus("chr1", 100, "GRCh38"), - hl.locus("chr1", 102, "GRCh38"), - includes_start=True, - includes_end=True, - ), - hl.interval( - hl.locus("chr2", 200, "GRCh38"), - hl.locus("chr2", 202, "GRCh38"), - includes_start=True, - includes_end=False, - ), - ] + """Test exploding an ArrayExpression of non-overlapping IntervalExpressions returns all expected loci.""" + intervals = hl.array( + [ + hl.interval( + hl.locus("chr1", 100, "GRCh38"), + hl.locus("chr1", 102, "GRCh38"), + includes_start=True, + includes_end=True, + ), + hl.interval( + hl.locus("chr2", 200, "GRCh38"), + hl.locus("chr2", 202, "GRCh38"), + includes_start=True, + includes_end=False, + ), + ] + ) result = explode_intervals_to_loci(intervals) loci = hl.eval(result) @@ -456,7 +458,7 @@ def test_explode_list_of_interval_expressions(self) -> None: def test_explode_table_overlapping_intervals_deduplicates(self) -> None: """Test that overlapping table intervals produce deduplicated loci when deduplicate=True.""" - # chr1:100-105 and chr1:103-108, both inclusive → without dedup: 12 rows + # chr1:100-105 and chr1:103-108, both inclusive -> without dedup: 12 rows # with deduplicate=True (default): 9 distinct loci [100..108] intervals = [ hl.interval( @@ -571,23 +573,25 @@ def test_explode_table_overlapping_intervals_keep_intervals_warns( ) def test_explode_list_overlapping_intervals_deduplicates(self) -> None: - """Test that overlapping intervals in a list are deduplicated when deduplicate=True.""" + """Test that overlapping intervals in an ArrayExpression are deduplicated when deduplicate=True.""" # Two overlapping intervals; deduplicate=True (default) removes duplicate # positions. - intervals = [ - hl.interval( - hl.locus("chr1", 100, "GRCh38"), - hl.locus("chr1", 105, "GRCh38"), - includes_start=True, - includes_end=True, - ), - hl.interval( - hl.locus("chr1", 103, "GRCh38"), - hl.locus("chr1", 108, "GRCh38"), - includes_start=True, - includes_end=True, - ), - ] + intervals = hl.array( + [ + hl.interval( + hl.locus("chr1", 100, "GRCh38"), + hl.locus("chr1", 105, "GRCh38"), + includes_start=True, + includes_end=True, + ), + hl.interval( + hl.locus("chr1", 103, "GRCh38"), + hl.locus("chr1", 108, "GRCh38"), + includes_start=True, + includes_end=True, + ), + ] + ) result = explode_intervals_to_loci(intervals, deduplicate=True) loci = hl.eval(result) @@ -598,27 +602,29 @@ def test_explode_list_overlapping_intervals_deduplicates(self) -> None: def test_explode_list_overlapping_intervals_no_deduplicate_warns( self, caplog ) -> None: - """Test that a warning is emitted and duplicates are present when deduplicate=False with overlapping list intervals.""" - intervals = [ - hl.interval( - hl.locus("chr1", 100, "GRCh38"), - hl.locus("chr1", 102, "GRCh38"), - includes_start=True, - includes_end=True, - ), - hl.interval( - hl.locus("chr1", 101, "GRCh38"), - hl.locus("chr1", 103, "GRCh38"), - includes_start=True, - includes_end=True, - ), - ] + """Test that a warning is emitted and duplicates are present when deduplicate=False with overlapping array intervals.""" + intervals = hl.array( + [ + hl.interval( + hl.locus("chr1", 100, "GRCh38"), + hl.locus("chr1", 102, "GRCh38"), + includes_start=True, + includes_end=True, + ), + hl.interval( + hl.locus("chr1", 101, "GRCh38"), + hl.locus("chr1", 103, "GRCh38"), + includes_start=True, + includes_end=True, + ), + ] + ) with caplog.at_level(logging.WARNING): result = explode_intervals_to_loci(intervals, deduplicate=False) assert ( - "Overlapping intervals in the input list may produce duplicate loci" + "Overlapping intervals in the input array may produce duplicate loci" in caplog.text ) loci = hl.eval(result) @@ -627,16 +633,15 @@ def test_explode_list_overlapping_intervals_no_deduplicate_warns( assert loci.count(hl.Locus("chr1", 101, "GRCh38")) == 2 assert loci.count(hl.Locus("chr1", 102, "GRCh38")) == 2 - def test_explode_empty_list_raises_type_error(self) -> None: - """Test that passing an empty Python list raises a TypeError. + def test_explode_plain_list_raises_assertion_error(self) -> None: + """Test that passing a plain Python list raises an AssertionError. - Hail cannot infer the element type of an empty Python list, so - ``hl.array([])`` (called internally) raises a TypeError. This is distinct - from passing a Hail-typed empty array expression, which would instead - fail the input-type assertion because ``ArrayExpression`` is not a - supported input type. + The API now requires a Hail ``ArrayExpression`` instead of a Python list. + Passing a Python list (including an empty one) fails the input-type + assertion. For an empty typed array use + ``hl.literal([], hl.tarray(hl.tinterval(hl.tlocus(...))))``). """ - with pytest.raises(TypeError): + with pytest.raises(AssertionError): explode_intervals_to_loci([]) def test_explode_list_cross_chromosome_same_position_deduplicates_correctly( @@ -648,20 +653,22 @@ def test_explode_list_cross_chromosome_same_position_deduplicates_correctly( distinct loci. Locus-based deduplication must preserve all 6 loci; a position-only deduplication strategy would incorrectly collapse them to 3. """ - intervals = [ - hl.interval( - hl.locus("chr1", 100, "GRCh38"), - hl.locus("chr1", 102, "GRCh38"), - includes_start=True, - includes_end=True, - ), - hl.interval( - hl.locus("chr2", 100, "GRCh38"), - hl.locus("chr2", 102, "GRCh38"), - includes_start=True, - includes_end=True, - ), - ] + intervals = hl.array( + [ + hl.interval( + hl.locus("chr1", 100, "GRCh38"), + hl.locus("chr1", 102, "GRCh38"), + includes_start=True, + includes_end=True, + ), + hl.interval( + hl.locus("chr2", 100, "GRCh38"), + hl.locus("chr2", 102, "GRCh38"), + includes_start=True, + includes_end=True, + ), + ] + ) result = explode_intervals_to_loci(intervals, deduplicate=True) loci = hl.eval(result) From ebc75dd27454a6611fb2f9c2e8619f1b10a9d78e Mon Sep 17 00:00:00 2001 From: Ruchit10 Date: Fri, 15 May 2026 19:14:21 -0400 Subject: [PATCH 48/48] Fix text assertion error --- tests/utils/test_interval_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/utils/test_interval_utils.py b/tests/utils/test_interval_utils.py index 10f51c581..c3f7a8a06 100644 --- a/tests/utils/test_interval_utils.py +++ b/tests/utils/test_interval_utils.py @@ -238,7 +238,7 @@ def test_explode_table_interval_field_wrong_type_raises_error( self, sample_interval_table: hl.Table ) -> None: """Test that a ValueError is raised when interval_field refers to a non-interval field.""" - with pytest.raises(ValueError, match="must be a hail interval"): + with pytest.raises(ValueError, match="must be an IntervalExpression"): explode_intervals_to_loci( sample_interval_table, interval_field="gene",