Repository navigation
Select only the data's contig from a multi-contig BED file #397
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
andrewkern
wants to merge
2
commits into
main
Choose a base branch
from
fix/363-multi-contig-bed
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,9 +4,12 @@ | |
| boolean array with offset-aware, O(1) range queries via prefix sums. | ||
| """ | ||
|
|
||
| import warnings | ||
|
|
||
| import cupy as cp | ||
| import numpy as np | ||
|
|
||
| from ._warnings import ContigNameWarning, _format_sample_list | ||
| from .zarr_io import parse_region | ||
|
|
||
|
|
||
|
|
@@ -141,15 +144,34 @@ def parse_bed(path, chrom=None): | |
| Path to a BED file (tab-delimited: chrom, start, end, ...). | ||
| Lines starting with '#', 'track', or 'browser' are skipped. | ||
| chrom : str, optional | ||
| If provided, only return intervals on this chromosome. | ||
| If provided, only return intervals on this chromosome. A name the | ||
| BED lacks matches a BED contig by name without a leading 'chr' and | ||
| case ('1' and 'chr1'), with a ContigNameWarning. | ||
|
|
||
| Returns | ||
| ------- | ||
| list of (str, int, int) | ||
| Parsed intervals as (chromosome, start, end) with 0-based half-open | ||
| coordinates. | ||
| """ | ||
| intervals = [] | ||
| return _read_bed(path, chrom)[0] | ||
|
|
||
|
|
||
| def _contig_key(name): | ||
| """A contig name without a leading 'chr' and without case, so the UCSC | ||
| name ('chr1') and the Ensembl or NCBI name ('1') of a contig match.""" | ||
| key = str(name).lower() | ||
| return key[3:] if key.startswith('chr') else key | ||
|
|
||
|
|
||
| def _read_bed(path, chrom=None): | ||
| """``(intervals, contigs)`` from one pass over a BED file: the intervals | ||
| on ``chrom`` (every interval when it is None), and every contig the BED | ||
| names, in file order. ``chrom`` matches by name as ``parse_bed`` says.""" | ||
| key = None if chrom is None else _contig_key(chrom) | ||
| contigs = {} | ||
| kept = [] | ||
| last, keep = None, False | ||
| with open(path) as f: | ||
| for line in f: | ||
| line = line.strip() | ||
|
|
@@ -161,10 +183,39 @@ def parse_bed(path, chrom=None): | |
| fields = line.split() | ||
| if len(fields) < 3: | ||
| continue | ||
| c, s, e = fields[0], int(fields[1]), int(fields[2]) | ||
| if chrom is None or c == chrom: | ||
| intervals.append((c, s, e)) | ||
| return intervals | ||
| c = fields[0] | ||
| # BED lines come grouped by contig, so the name test runs once | ||
| # per contig, not once per line. | ||
| if c != last: | ||
| last = c | ||
| contigs[c] = None | ||
| keep = key is None or _contig_key(c) == key | ||
| if keep: | ||
| kept.append((c, int(fields[1]), int(fields[2]))) | ||
| if chrom is not None: | ||
| chrom = str(chrom) | ||
| names = list(dict.fromkeys(c for c, _, _ in kept)) | ||
| if chrom not in names and len(names) == 1: | ||
| warnings.warn( | ||
| f"BED contig {names[0]!r} does not match data contig " | ||
| f"{chrom!r} exactly; inferring that they are the same contig.", | ||
| ContigNameWarning, stacklevel=3) | ||
| chrom = names[0] | ||
| kept = [iv for iv in kept if iv[0] == chrom] | ||
| return kept, list(contigs) | ||
|
|
||
|
|
||
| def _contig_intervals(path, chrom): | ||
| """``_read_bed(path, chrom)``, but a BED with more than one contig raises | ||
| ValueError when ``chrom`` is None: merging them would count other | ||
| contigs' intervals as accessible bases of the data.""" | ||
| intervals, contigs = _read_bed(path, chrom) | ||
| if chrom is None and len(contigs) > 1: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm... say you're loading from zarr. Neither from pg_gpu import HaplotypeMatrix
# vcz_store spans 2 contigs; bed has intervals on 2 contigs; no region given.
hm = HaplotypeMatrix.from_zarr(vcz_store, accessible_bed=two_contig_bed)
# ValueError: BED file ... has intervals on 2 contigs (...);
# name the contig of the data with chrom= or region=.
hm = HaplotypeMatrix.from_zarr(vcz_store, accessible_bed=two_contig_bed, chrom="1") # following the hint
# TypeError: from_zarr() got an unexpected keyword argument 'chrom'so that needs special casing to the error or adding a chrom arg for |
||
| raise ValueError( | ||
| f"BED file {path} has intervals on {len(contigs)} contigs " | ||
| f"({_format_sample_list(contigs, range(len(contigs)), limit=5)}); " | ||
| "name the contig of the data with chrom= or region=.") | ||
| return intervals, contigs | ||
|
|
||
|
|
||
| def bed_to_mask(path, chrom, length, offset=0): | ||
|
|
@@ -174,8 +225,9 @@ def bed_to_mask(path, chrom, length, offset=0): | |
| ---------- | ||
| path : str or path-like | ||
| Path to a BED file defining accessible regions. | ||
| chrom : str | ||
| Chromosome name to extract from the BED file. | ||
| chrom : str or None | ||
| Chromosome name to extract from the BED file. None is allowed only | ||
| for a BED with one contig. | ||
| length : int | ||
| Total length of the mask array (number of genomic positions to cover). | ||
| offset : int | ||
|
|
@@ -193,7 +245,12 @@ def bed_to_mask(path, chrom, length, offset=0): | |
| positions s+1 through e (inclusive). Mask index k represents 1-based | ||
| position offset + k. | ||
| """ | ||
| intervals = parse_bed(path, chrom=chrom) | ||
| return _intervals_to_mask(_contig_intervals(path, chrom)[0], length, offset) | ||
|
|
||
|
|
||
| def _intervals_to_mask(intervals, length, offset): | ||
| """AccessibleMask of ``length`` bases from ``offset`` with ``intervals`` | ||
| (0-based half-open BED records) set True.""" | ||
| mask = np.zeros(length, dtype=bool) | ||
| for _, s, e in intervals: | ||
| # BED (s, e) [0-based half-open] = 1-based positions s+1..e | ||
|
|
@@ -218,7 +275,8 @@ def resolve_accessible_mask(mask_or_path, chrom_start, chrom_end, chrom=None): | |
| silently dropping BED accessible bases that fall outside the | ||
| matrix's variant range (relevant for variants-only VCFs). | ||
| chrom : str, optional | ||
| Chromosome name (required for BED file input). | ||
| Chromosome name. Required for a BED file with more than one contig; | ||
| a one-contig BED needs none. | ||
|
|
||
| Returns | ||
| ------- | ||
|
|
@@ -232,10 +290,12 @@ def resolve_accessible_mask(mask_or_path, chrom_start, chrom_end, chrom=None): | |
| # Treat as file path. Build the mask over the union of [chrom_start, | ||
| # chrom_end] and the BED's own extent so no accessible bases are lost | ||
| # when the matrix's variant range is narrower than the BED. | ||
| intervals = parse_bed(mask_or_path, chrom=chrom) | ||
| intervals, contigs = _contig_intervals(mask_or_path, chrom) | ||
| if not intervals: | ||
| where = "" if chrom is None else f" for contig {str(chrom)!r}" | ||
| raise ValueError( | ||
| f"No BED intervals found for chrom={chrom!r} in {mask_or_path}") | ||
| f"No BED intervals found{where} in {mask_or_path}; its contigs " | ||
| f"are: {_format_sample_list(contigs, range(len(contigs)), limit=5) or 'none'}.") | ||
| # 1-based inclusive bounds of the BED (BED is 0-based half-open: a record | ||
| # (s, e) covers 1-based positions s+1..e). | ||
| bed_min = min(s + 1 for _, s, _ in intervals) | ||
|
|
@@ -248,7 +308,7 @@ def resolve_accessible_mask(mask_or_path, chrom_start, chrom_end, chrom=None): | |
| if length <= 0: | ||
| raise ValueError( | ||
| "Could not determine mask range from BED and chromosome bounds") | ||
| return bed_to_mask(mask_or_path, chrom=chrom, length=length, offset=offset) | ||
| return _intervals_to_mask(intervals, length, offset) | ||
|
|
||
|
|
||
| def slice_fields(fields, keep_idx, accessible_idx=None): | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the stacklevel right here? It's correct for
parse_bed(path, chrom=...) -> _read_bed, but there's other entry points that are deeper. For example,resolve_accessible_mask(...) -> _contig_intervals -> read_bedneeds stacklevel=4. I'm not clear on what the intended behaviour is here.