Add function for exploding intervals to loci by Ruchit10 · Pull Request #789 · broadinstitute/gnomad_methods

Ruchit10 · 2025-07-03T19:18:42Z

Feature Addition:

Added a function in intervals.py which explodes hail intervals into per base loci, keeping same reference genome as original interval.

Optionally it can return the original interval that the locus belongs to or drop it from the exploded table.

ch-kr

thanks for adding! a few suggestions

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

…t and end

ch-kr

a few more minor changes -- sorry about the back and forth on the MT part!

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

ch-kr

thank you for adding additional flexibility! I've added some suggestions to streamline the code (and hopefully didn't break the formatting too much)

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

…ntation

ch-kr · 2025-09-16T17:45:52Z

a little late, but I also forgot that we've been requesting that each person who adds or changes code in this repo add tests (https://github.com/broadinstitute/gnomad_methods/tree/main/tests). could you add tests for this new function?

ch-kr

thanks for adding tests! a few more thoughts

ch-kr · 2026-04-24T20:07:40Z

+    @pytest.fixture
+    def sample_interval_expr(self):
+        """Fixture to create a sample interval expression."""
+        return hl.literal(


do you need this literal? hl.interval returns an IntervalExpression

Ah this is very silly on my end. I was using hl.Interval instead of hl.interval before so was wondering why the interval function wasn't working (which is reason for adding hl.literal). Will change this

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

ch-kr

a few more comments

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

ch-kr

a few more style comments

ch-kr · 2026-05-15T17:53:13Z

+            "Input is a list of IntervalExpressions, so function will return an"
+            " ArrayExpression of loci within all input intervals."


should these be consistent types? the current behavior means the user passes in a list but receives an ArrayExpression

I feel an ArrayExpression as input makes more sense in that case. That will be the most common input type in hail isn't it?

ch-kr · 2026-05-15T17:58:25Z

+    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,
+        )
+    )


this can be replaced with a call to _make_loci_array

ch-kr · 2026-05-15T18:00:24Z

+            )
+
+        loci_arrays = [_make_loci_array(i) for i in intervals]
+        if not flatten:


for consistency, this probably should be reordered to be if flatten:

ch-kr · 2026-05-15T18:01:32Z

+        intervals = intervals.key_by(
+            locus=hl.locus(
+                intervals[interval_field].start.contig,
+                intervals._pos,
+                reference_genome=get_reference_genome(intervals[interval_field]),
+            )
+        )


this can also be replaced by a call to _make_loci_array

ch-kr · 2026-05-15T18:02:29Z

+                    " intervals overlap."
+                )
+            else:
+                intervals = intervals.distinct()


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?

ch-kr · 2026-05-15T18:05:02Z

+        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


should there be a check to make sure the positions are sorted?

ch-kr · 2026-05-15T18:09:21Z

+            result = hl.array(hl.set(result))
+        return result
+
+    intervals_expr = (


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

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

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

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

ch-kr

a few more comments after revisiting

ch-kr · 2026-07-14T14:43:55Z

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


Suggested change

.. warning::

- Overlapping intervals will produce duplicate loci. Use ``deduplicate=True``

.. warning::

- Overlapping intervals will produce duplicate loci. Use ``deduplicate=True``

nit

ch-kr · 2026-07-14T14:44:56Z

+          chromosomes), as it will create extremely large arrays, which may cause
+          performance issues.
+
+    Note that intervals that cross chromosomes are currently not supported.


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

ch-kr · 2026-07-14T15:42:02Z

+        " IntervalExpressions!"
+    )
+
+    if isinstance(intervals, hl.Table) and (


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

ch-kr · 2026-07-14T15:44:58Z

+            result = hl.array(hl.set(result))
+        return result
+
+    intervals_expr = (


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

ch-kr · 2026-07-14T15:47:05Z

+    interval_field: Optional[str] = None,
+    keep_intervals: Optional[bool] = False,
+    deduplicate: bool = True,
+    flatten: bool = True,


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

ch-kr · 2026-07-14T15:49:44Z

+            result = hl.flatten(loci_arrays)
+            if deduplicate:
+                result = hl.array(hl.set(result))
+            result = hl.sorted(result)


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

ch-kr · 2026-07-14T15:51:08Z

+
+        intervals = intervals.drop(*fields_to_drop)
+
+        if deduplicate:


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

ch-kr · 2026-07-14T15:52:30Z

+        interval = hl.interval(
+            hl.locus("chr1", 100, "GRCh38"),
+            hl.locus("chr1", 105, "GRCh38"),
+            includes_start=False,
+            includes_end=False,
+        )


this looks like the same interval that is built into the function above. could this be extracted into a fixture?

ch-kr · 2026-07-14T15:54:02Z

+        ht = hl.Table.parallelize(
+            [{"interval": interval, "gene": "GENE1"}],
+            hl.tstruct(interval=hl.tinterval(hl.tlocus("GRCh38")), gene=hl.tstr),
+        )


there are a few hl.Table.parallelizes in the tests as well, could you update the fixture or create other fixtures to reduce redundancy?

ch-kr · 2026-07-14T16:00:11Z

+        # Verify that other fields are still present.
+        assert all(hasattr(row, "gene") for row in result)
+
+    def test_explode_interval_expression(


the multiple test_explode_interval_expression* tests that only change testing whether the start/end are included/excluded can be collapsed into a single@pytest.mark.parametrized function that uses a table with a few rows that vary the start/end inclusion. this is also true of the test_explode_table_*excludes* tests

Add function for exploding intervals to loci

67d65a1

Ruchit10 requested a review from a team as a code owner July 3, 2025 19:18

ch-kr requested changes Jul 3, 2025

View reviewed changes

Comment thread gnomad/utils/intervals.py Outdated

Comment thread gnomad/utils/intervals.py Outdated

Comment thread gnomad/utils/intervals.py Outdated

Comment thread gnomad/utils/intervals.py

Ruchit10 and others added 2 commits July 3, 2025 17:09

Update gnomad/utils/intervals.py

9b2cae2

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Update gnomad/utils/intervals.py

31eeac1

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

ch-kr added the Changelog: new feature label Jul 9, 2025

Ruchit10 added 2 commits July 10, 2025 15:24

Add flexibility to accept MT/HT and adjust to interval including star…

b4214e5

…t and end

Black reformatting

e870d3b

Ruchit10 requested a review from ch-kr July 10, 2025 19:47

ch-kr reviewed Jul 11, 2025

View reviewed changes

Ruchit10 and others added 13 commits July 11, 2025 17:14

Update gnomad/utils/intervals.py

f8d53cb

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Update gnomad/utils/intervals.py

3e96a21

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Update gnomad/utils/intervals.py

40e2702

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Update gnomad/utils/intervals.py

53b50d7

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Update gnomad/utils/intervals.py

3d6774d

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Update gnomad/utils/intervals.py

238f547

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Update gnomad/utils/intervals.py

6194688

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Update gnomad/utils/intervals.py

fcf450e

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Update gnomad/utils/intervals.py

77c9f2c

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Update gnomad/utils/intervals.py

caf9dab

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Update gnomad/utils/intervals.py

c2dc321

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Change explode_intervals_to_loci to accept interval expression or HT

f4b28a5

Fix pre-commit

1162c19

Ruchit10 requested a review from ch-kr July 14, 2025 17:22

ch-kr reviewed Jul 14, 2025

View reviewed changes

Ruchit10 and others added 5 commits July 15, 2025 11:13

Update gnomad/utils/intervals.py

1927ed6

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Update gnomad/utils/intervals.py

91a4948

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Update gnomad/utils/intervals.py

defa0b0

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Update gnomad/utils/intervals.py

078ac91

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Update gnomad/utils/intervals.py

020b493

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Ruchit10 and others added 3 commits July 15, 2025 11:18

Update gnomad/utils/intervals.py

ec2cb97

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Update gnomad/utils/intervals.py

f8864f7

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

update docstrings, add logging and typing imports, fix typos and inde…

6216cee

…ntation

Ruchit10 added 4 commits February 2, 2026 17:21

Added tests for interval to loci explode function

6921adc

Reformat test script using black

5f852fc

Fixed failing tests part 1

fde2422

Fixed intervals assertion in explode_intervals_to_loci

ccd07fb

Ruchit10 requested a review from ch-kr April 24, 2026 19:15

ch-kr reviewed Apr 24, 2026

View reviewed changes

Ruchit10 and others added 7 commits April 24, 2026 17:52

Update gnomad/utils/intervals.py

31a307b

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Update gnomad/utils/intervals.py

de65275

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Apply suggestions from code review

9b6b52d

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Changes addressed per PR comments and added more tests

25be488

Add back doc strings in test functions to fix pylint errors

940a220

Fix pylint attempt 2

abe4557

Fix test errors attempt 3

19a624b

ch-kr reviewed Apr 29, 2026

View reviewed changes

Ruchit10 and others added 7 commits April 29, 2026 16:14

Apply suggestions from code review

d6645ce

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Addressing comments (round3) and adding more tests

9082e06

Fix pylint issues

435c1d9

fix testing errors

4a410a3

Change error messaging for interval expression check

e13373c

Black formatting

0fe4259

Change test assertion to match error message changes

658fbd2

ch-kr reviewed May 15, 2026

View reviewed changes

Ruchit10 and others added 3 commits May 15, 2026 14:35

Apply suggestions from code review

8be66f6

Co-authored-by: Katherine Chao <kchao@broadinstitute.org>

Address comments (round4) and adjust tests accordingly

e9ef49b

Fix text assertion error

ebc75dd

ch-kr requested changes Jul 14, 2026

View reviewed changes

		"Input is a list of IntervalExpressions, so function will return an"
		" ArrayExpression of loci within all input intervals."

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

Uh oh!

Conversation

Ruchit10 commented Jul 3, 2025

Uh oh!

ch-kr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ch-kr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ch-kr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ch-kr commented Sep 16, 2025

Uh oh!

ch-kr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ch-kr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ch-kr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment