Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
67d65a1
Add function for exploding intervals to loci
Ruchit10 Jul 3, 2025
9b2cae2
Update gnomad/utils/intervals.py
Ruchit10 Jul 3, 2025
31eeac1
Update gnomad/utils/intervals.py
Ruchit10 Jul 3, 2025
b4214e5
Add flexibility to accept MT/HT and adjust to interval including star…
Ruchit10 Jul 10, 2025
e870d3b
Black reformatting
Ruchit10 Jul 10, 2025
f8d53cb
Update gnomad/utils/intervals.py
Ruchit10 Jul 11, 2025
3e96a21
Update gnomad/utils/intervals.py
Ruchit10 Jul 11, 2025
40e2702
Update gnomad/utils/intervals.py
Ruchit10 Jul 11, 2025
53b50d7
Update gnomad/utils/intervals.py
Ruchit10 Jul 11, 2025
3d6774d
Update gnomad/utils/intervals.py
Ruchit10 Jul 11, 2025
238f547
Update gnomad/utils/intervals.py
Ruchit10 Jul 11, 2025
6194688
Update gnomad/utils/intervals.py
Ruchit10 Jul 11, 2025
fcf450e
Update gnomad/utils/intervals.py
Ruchit10 Jul 11, 2025
77c9f2c
Update gnomad/utils/intervals.py
Ruchit10 Jul 11, 2025
caf9dab
Update gnomad/utils/intervals.py
Ruchit10 Jul 11, 2025
c2dc321
Update gnomad/utils/intervals.py
Ruchit10 Jul 11, 2025
f4b28a5
Change explode_intervals_to_loci to accept interval expression or HT
Ruchit10 Jul 14, 2025
1162c19
Fix pre-commit
Ruchit10 Jul 14, 2025
1927ed6
Update gnomad/utils/intervals.py
Ruchit10 Jul 15, 2025
91a4948
Update gnomad/utils/intervals.py
Ruchit10 Jul 15, 2025
defa0b0
Update gnomad/utils/intervals.py
Ruchit10 Jul 15, 2025
078ac91
Update gnomad/utils/intervals.py
Ruchit10 Jul 15, 2025
020b493
Update gnomad/utils/intervals.py
Ruchit10 Jul 15, 2025
04edebc
Update gnomad/utils/intervals.py
Ruchit10 Jul 15, 2025
ec2cb97
Update gnomad/utils/intervals.py
Ruchit10 Jul 15, 2025
f8864f7
Update gnomad/utils/intervals.py
Ruchit10 Jul 15, 2025
6216cee
update docstrings, add logging and typing imports, fix typos and inde…
Ruchit10 Jul 15, 2025
6921adc
Added tests for interval to loci explode function
Ruchit10 Feb 2, 2026
5f852fc
Reformat test script using black
Ruchit10 Feb 6, 2026
fde2422
Fixed failing tests part 1
Ruchit10 Apr 24, 2026
ccd07fb
Fixed intervals assertion in explode_intervals_to_loci
Ruchit10 Apr 24, 2026
31a307b
Update gnomad/utils/intervals.py
Ruchit10 Apr 24, 2026
de65275
Update gnomad/utils/intervals.py
Ruchit10 Apr 24, 2026
9b6b52d
Apply suggestions from code review
Ruchit10 Apr 24, 2026
25be488
Changes addressed per PR comments and added more tests
Ruchit10 Apr 27, 2026
940a220
Add back doc strings in test functions to fix pylint errors
Ruchit10 Apr 27, 2026
abe4557
Fix pylint attempt 2
Ruchit10 Apr 27, 2026
19a624b
Fix test errors attempt 3
Ruchit10 Apr 27, 2026
d6645ce
Apply suggestions from code review
Ruchit10 Apr 29, 2026
9082e06
Addressing comments (round3) and adding more tests
Ruchit10 May 4, 2026
435c1d9
Fix pylint issues
Ruchit10 May 4, 2026
4a410a3
fix testing errors
Ruchit10 May 5, 2026
e13373c
Change error messaging for interval expression check
Ruchit10 May 15, 2026
0fe4259
Black formatting
Ruchit10 May 15, 2026
658fbd2
Change test assertion to match error message changes
Ruchit10 May 15, 2026
8be66f6
Apply suggestions from code review
Ruchit10 May 15, 2026
e9ef49b
Address comments (round4) and adjust tests accordingly
Ruchit10 May 15, 2026
ebc75dd
Fix text assertion error
Ruchit10 May 15, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
182 changes: 181 additions & 1 deletion gnomad/utils/intervals.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
# noqa: D100

from typing import List, Union
import logging
from typing import List, Optional, Union

import hail as hl

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]):
"""
Expand Down Expand Up @@ -112,3 +120,175 @@ def _add_padding(
return [_add_padding(i) for i in intervals]
else:
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(
Comment thread
ch-kr marked this conversation as resolved.
Comment thread
Ruchit10 marked this conversation as resolved.
intervals: Union[hl.Table, hl.expr.IntervalExpression, hl.expr.ArrayExpression],
interval_field: Optional[str] = None,
keep_intervals: Optional[bool] = False,
deduplicate: bool = True,
flatten: bool = True,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there doesn't seem to be a test covering flatten=False in the test suite

) -> Union[hl.Table, hl.expr.ArrayExpression]:
"""
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 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``
Comment on lines +165 to +166

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.. warning::
- Overlapping intervals will produce duplicate loci. Use ``deduplicate=True``
.. warning::
- Overlapping intervals will produce duplicate loci. Use ``deduplicate=True``

nit

(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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we should raise an error if a user passes an interval that crosses chromosomes?


: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, hl.expr.ArrayExpression)
), (
"Input must be a Table, IntervalExpression, or ArrayExpression of"
" IntervalExpressions!"
)

if isinstance(intervals, hl.Table) and (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seeing this function again with fresh eyes, you could reorder how you tackle the different inputs (Table, IntervalExpression, ArrayExpression). the current function structure checks whether the input is a Table 3 separate times, and reordering would remove the extra checks.

by reordering, I mean this function could handle the case of an input IntervalExpression + return, then the case of an input ArrayExpression + return, and finally the case of an input Table

not interval_field or keep_intervals is None
):
raise ValueError(
"`interval_field` and `keep_intervals` must be defined if input is a Table!"
)
if isinstance(intervals, hl.Table):
if interval_field not in intervals.row:
raise ValueError(
Comment thread
Ruchit10 marked this conversation as resolved.
"`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 be an IntervalExpression"
" in the input Table!"
)

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 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 array may produce duplicate loci"
" in the returned ArrayExpression. Set `deduplicate=True` to remove"
" them."
)

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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should add a test checking that results are sorted when flatten is True

return result
else:
return loci_arrays

intervals_expr = (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks like this expression only gets used once (it doesn't get used if this is a Table below); is this code necessary?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also gets used in a table to annotate _pos, but it gets wrapped into interval_to_pos_range either way so maybe I should make that a single call upstream

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what I meant when I wrote this comment was that you could directly call _make_loci_array on intervals[interval_field], since the only case that is handled below is if isinstance(intervals, hl.Table). however, if you reorder the code as suggested above, you shouldn't need this if/else at all

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(_loci=loci_array).explode("_loci")
intervals = intervals.key_by(locus=intervals._loci)

fields_to_drop = ["_loci"]
if not keep_intervals:
fields_to_drop.append(interval_field)

intervals = intervals.drop(*fields_to_drop)

if deduplicate:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the other logic branch (deduplicate=False, keep_intervals=True) case also be covered in tests?

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()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we add a warning here that distinct() arbitrarily deduplicates, which means that if the input table was annotated with something like gene or transcript ID, this will pick one at random for duplicated loci?

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

logger.info(
"Input is an IntervalExpression, so function will return an ArrayExpression of"
" loci within the input interval."
)
return loci_array
Loading
Loading