Skip to content

Tuple to dataclass - #2

Open
XLIU-hub wants to merge 223 commits into
masterfrom
tuple_to_dataclass
Open

Tuple to dataclass#2
XLIU-hub wants to merge 223 commits into
masterfrom
tuple_to_dataclass

Conversation

@XLIU-hub

@XLIU-hub XLIU-hub commented Mar 27, 2026

Copy link
Copy Markdown
Contributor

Pull Request Details

Refactor the crossmapper position API from tuple-based values to typed dataclasses, adding validation for dataclass values and class initialization inputs.

  • Add Coord, Locus.Point, MultiLocus.Point, GenomicPoint, NonCodingPoint, CodingPoint, and ProteinPoint dataclasses.
  • Update locus, multi-locus, genomic, non-coding, coding, and protein conversions to accept and return the new data models.
  • Add validation for position values, offsets, regions, loci, CDS ranges, and reference-length bounds.
  • Expand and update tests for validation errors.
  • Add typings for all functions.

Breaking Changes:

  • Callers must replace tuple position inputs and outputs with the relevant dataclass instances.
  • Public dataclass inputs and internal conversion models now raise errors for invalid values.

Validation

  • Updated tests for dataclass inputs and outputs for locus, multi_locus, and crossmapper.
  • Added coverage for validation errors and error messages.
  • Added CI checks for typing compatibility.

Xiaoyun Liu added 30 commits February 19, 2026 16:16
Comment thread mutalyzer_crossmapper/locus.py Outdated
raise IndexError(f'Position {position} exceeds locus length.')

:arg int coordinate: Coordinate.
def to_position(self, coord: Coord) -> Point:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Change function name to to_point ?

self._inverted = inverted
self.boundary = location[0], location[1] - 1
self._end = self.boundary[1] - self.boundary[0]
self._end = location[1] - location[0] # one-based length of the locus

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Maybe better with name self._range?

Comment thread mutalyzer_crossmapper/multi_locus.py Outdated

def __post_init__(self) -> None:
LocusPoint.__post_init__(self)
if self.region not in ("", "u", "d"):

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Use ''.

self._orientation = -1 if inverted else 1
self._offsets = _offsets(locations, self._orientation)
# one-based length of the MultiLocus
self._end = sum(end - start for start, end in locations)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

self._end or self._range


def _direction(self, index):
def _validate_point(self, index: int, position: int, offset: int, region: str) -> None:
"""Validate if a multi locus Point is valid under HGVS rules.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Validate the position, offset and region values relatively to multi locus information.

:arg int offset: Offset.
:arg str region: Region.
"""
if region == 'u':

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

How about split the function to make it less complex.

if offset != 0 and position not in (0, self._end - 1):
raise ValueError(f'Position {position} is not at a locus boundary.')
if offset < 0 and position != 0:
raise IndexError(f'Offset {offset} should be at a locus start.')

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

IndexError or ValueError

_check_int(self.offset)
if self.region not in self.allowed_regions:
raise ValueError(
f'Region {self.region} is invalid, it must be a string from {self.allowed_regions}.'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Display the region strings without list.

Comment thread mutalyzer_crossmapper/crossmapper.py Outdated

b0 = self._noncoding.to_position(cds[0])
b1 = self._noncoding.to_position(cds[1])
cds_start = self._noncoding.to_position(Coord(cds[0]))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

coding_start/end?

Comment thread mutalyzer_crossmapper/crossmapper.py Outdated
b1 = self._noncoding.to_position(cds[1])
cds_start = self._noncoding.to_position(Coord(cds[0]))
cds_end = self._noncoding.to_position(Coord(cds[1] - 1))
exon_start = self._noncoding.to_position(Coord(locations[0][0]))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

transcript_start/end?

region = noncoding_point.region

if region == 'u':
if self._exons[0] == self._coding[0]:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

explain for missing 5'/ 3' UTR

if region == 'u':
position = 1
if region == 'd':
if self._coding[1] == self._exons[1]:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

if position in upstream and downstream has been checked at the crossmapper level, maybe the downstream position assigning can be simplified as position = self._exons[1]? the same as the upstream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants