chore: add lower, lift, intersect, dna, expand_coord DIS methods by SophiaPerzan-DG · Pull Request #229 · deepgenomics/GenomeKit

SophiaPerzan-DG · 2026-05-08T14:16:19Z

also:

chore: remove index direction toggle in DIS

… remove index direction toggle

Copilot

Pull request overview

This PR extends DisjointIntervalSequence (DIS) with additional coordinate/interval manipulation and mapping utilities (including genomic ↔ DIS coordinate conversion and DNA extraction), updates documentation accordingly, and adds tests for the new behaviors. It also removes the prior DIS “index direction” toggle and introduces a Genome.dna(...) convenience path intended to accept a DIS directly.

Changes:

Added DIS methods: expand_coord, _lower_coord, lower, genomic_span, _lift_position, lift_interval, intersect, and dna; removed the global index-direction toggle.
Updated Genome.dna behavior to dispatch when passed a DisjointIntervalSequence.
Added/expanded unit tests and documentation for the new DIS coordinate mapping and DNA behaviors.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/test_genome.py	Adds coverage for `Genome.dna()` when called with an `Interval` vs a DIS.
tests/test_diseq.py	Adds extensive tests for DIS coordinate expansion, lowering/lifting, intersection, span, and DNA.
genome_kit/genome.py	Changes `Genome.dna` implementation to dispatch for DIS inputs.
genome_kit/diseq.py	Implements new DIS methods (expand/lower/lift/intersect/dna) and removes index-direction configuration.
genome_kit/init.py	Exposes `DisjointIntervalSequence` in `__all__`.
docs-src/diseq.rst	Documents `expand_coord`, coordinate mapping (`lower`/`lift_interval`), and DIS DNA behavior (including `Genome.dna(dis)` convenience).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

This reverts commit 572bc63.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

docs-src/diseq.rst:819

The docs claim Genome.dna accepts a DIS directly (genome.dna(dis)), but GenomeDNA.__call__ only accepts Interval-typed arguments via PyAsInterval (see src/py_interval.h). Unless there’s an unshown overload, this example will raise TypeError. Either remove this claim/example or implement DIS dispatch in the DNA API.

For convenience, :py:meth:`Genome.dna <genome_kit.Genome.dna>` accepts a
DIS directly and dispatches to ``DisjointIntervalSequence.dna``, so
either of the following is equivalent::

    >>> dis.dna()
    >>> genome.dna(dis)

ovesh · 2026-05-11T20:21:09Z

+        5' -> 3' when ``coord`` falls exactly on an internal boundary between two
+        adjacent coord intervals — where ``iv_upstream_coord`` is the upstream-most
+        coordinate boundary, and ``iv_downstream_coord`` is the downstream-most
+        coordinate boundary (regardless of strand). Touching intervals produce


I forgot that we don't normalize user input when they create a DIS with 2 adjacent intervals. Shouldn't GK normalize to a single interval internally?

@AliceDG any reason to conserve user input in this scenario?

Adjacent here means "the next coordinate interval in the list of coordinate intervals" not that they are adjacent in genomic space. A 2-element list would be returned even if there was a gap between two coord intervals (in genomic space)

Yeah, my concern is not so relevant to this PR but more general. I was referring to "touching" rather than "adjacent".

ovesh · 2026-05-11T20:26:50Z

+        ``dnstream`` bases. The segment is expanded an equal amount in the
+        upstream/downstream direction as the coordinate intervals.


What happens when the segment isn't adjacent to the coord system edge?

it still get's expanded. So if the segment is inset 10bp on both 5' and 3' ends and then you expand the coordinate, it will remain so after the expansion. This also means if you have a 0-length interval and expand the coordinate space, the segment will no longer be 0-length after expansion.

@AliceDG is this the desired behavior?
(this comment is not blocking)

ovesh

approved, let's wait a bit before merging to allow @AliceDG to comment

s22chan · 2026-05-18T05:26:46Z

    DIS Coordinates:       0   1   2   3   4   5   6   7
                                  Opposite Strand

+::


Suggested change

::

.. code-block:: python

s22chan · 2026-05-18T05:29:54Z

+
+``lower`` projects the segment back to genomic space. ``lift_interval``
+projects a genomic interval into the DIS's index space and clips it
+against the segment. They are conceptual inverses, but each has to deal


are they mechanically inverses as well? I remember the old code merged adjacent intervals. this one returns in sorted order?

s22chan · 2026-05-18T05:33:19Z

+lift_interval
+~~~~~~~~~~~~~
+
+``lift_interval`` is the inverse: it takes a genomic


Suggested change

``lift_interval`` is the inverse: it takes a genomic

:py:meth:`~genome_kit.DisjointIntervalSequence.lift_interval` is the inverse: it takes a genomic

and etc below

s22chan · 2026-05-18T05:38:07Z

+
+        Parameters
+        ----------
+        coord : :py:class:`int`


Suggested change

coord : :py:class:`int`

coord

sphinx can parse out the type annotations, no need to repeat (and possibly have a conflict)

s22chan · 2026-05-18T05:41:28Z

+        Args:
+            upstream: Bases to add at the coordinate space's 5' end.
+            dnstream: Bases to add at the coordinate space's 3' end.
+                Defaults to upstream (symmetric).
+
+        Raises:
+            ValueError: If either argument is negative.


Mixing google style docstrings with numpy ones

s22chan · 2026-05-18T05:43:55Z

+        iv0, ivn = coord_ivs[0], coord_ivs[-1]
+        chrom = iv0.chromosome
+        refg = iv0.reference_genome
+        if self.coord_strand == "+":
+            if len(coord_ivs) == 1:
+                coord_ivs = [Interval(chrom, "+", iv0.start - upstream, iv0.end + dnstream, refg)]
+            else:
+                coord_ivs[0] = Interval(chrom, "+", iv0.start - upstream, iv0.end, refg)
+                coord_ivs[-1] = Interval(chrom, "+", ivn.start, ivn.end + dnstream, refg)
+        else:
+            if len(coord_ivs) == 1:
+                coord_ivs = [Interval(chrom, "-", iv0.start - dnstream, iv0.end + upstream, refg)]
+            else:
+                coord_ivs[0] = Interval(chrom, "-", iv0.start, iv0.end + upstream, refg)
+                coord_ivs[-1] = Interval(chrom, "-", ivn.start - dnstream, ivn.end, refg)


is it possible to just use the iv0/ivn.expand function here. Less math (which is the point of gk)

s22chan · 2026-05-18T05:47:59Z

    on_coordinate_strand: bool


 class DisjointIntervalSequence:


is a DIS supposed to handle the same protocol as a regular interval? EG, you don't need to if/else on the type. If so, I think maybe there's some missing methods like spanning/midpoint/distance etc

SophiaPerzan-DG added 5 commits May 8, 2026 10:33

implement lower()

ac64101

implement dna(), lift(), intersect(), genomic_span(), expand_coord(),…

4c88f50

… remove index direction toggle

add lift_interval()

9b53737

add documentation

a305fab

add genome.dna() support for DIS

572bc63

SophiaPerzan-DG force-pushed the diseq-pt3 branch from 5f83cca to 572bc63 Compare May 8, 2026 14:36

SophiaPerzan-DG requested a review from Copilot May 8, 2026 15:00

Copilot started reviewing on behalf of SophiaPerzan-DG May 8, 2026 15:01 View session

Copilot AI reviewed May 8, 2026

View reviewed changes

Comment thread genome_kit/genome.py Outdated

SophiaPerzan-DG added 2 commits May 8, 2026 11:43

fix unit test errors

4012afc

Revert "add genome.dna() support for DIS"

2e32278

This reverts commit 572bc63.

SophiaPerzan-DG requested a review from Copilot May 8, 2026 17:29

Copilot started reviewing on behalf of SophiaPerzan-DG May 8, 2026 17:30 View session

Copilot AI reviewed May 8, 2026

View reviewed changes

Comment thread genome_kit/diseq.py

Comment thread genome_kit/diseq.py Outdated

Comment thread genome_kit/diseq.py

SophiaPerzan-DG marked this pull request as ready for review May 8, 2026 17:57

SophiaPerzan-DG added 3 commits May 8, 2026 14:07

fix docstring

b5d237d

remove old reference to IndexDirection enum

b0785e0

fix dob build errors

2922725

SophiaPerzan-DG requested review from ovesh and s22chan May 8, 2026 18:16

ovesh reviewed May 11, 2026

View reviewed changes

SophiaPerzan-DG added 2 commits May 11, 2026 17:14

remove unneeded coord_end properties

83c3660

add examples in docs

02569ce

ovesh approved these changes May 12, 2026

View reviewed changes

s22chan reviewed May 18, 2026

View reviewed changes

		``dnstream`` bases. The segment is expanded an equal amount in the
		upstream/downstream direction as the coordinate intervals.

	``lift_interval`` is the inverse: it takes a genomic
	:py:meth:`~genome_kit.DisjointIntervalSequence.lift_interval` is the inverse: it takes a genomic

Conversation

SophiaPerzan-DG commented May 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

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!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ovesh left a comment

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

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

s22chan May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

s22chan May 18, 2026 •

edited

Loading