Skip to content

Tuple to dataclass#2

Open
XLIU-hub wants to merge 116 commits intomasterfrom
tuple_to_dataclass
Open

Tuple to dataclass#2
XLIU-hub wants to merge 116 commits intomasterfrom
tuple_to_dataclass

Conversation

@XLIU-hub
Copy link
Copy Markdown
Contributor

Pull Request Details

This pull request replaces tuple-based return values with dictionaries across the crossmapper. This change introduces dictionaries with explicit keys, improving:

  • Readability of the codebase
  • API clarity for users, especially in documentation

For example, instead of:
position, offset, in_cds, in_transcript = func(...)
The API now returns:
pos_m = func(...)
pos_m['position']
pos_m['offset']
pos_m['region']
pos_m['position_in_codon']

Documentation has been updated to reflect these new return types.

Breaking Changes

Functions that previously returned positions in tuples now return dictionaries. This affects downstream code that:

  • Relies on tuple unpacking
  • Access values by index
  • Use position values without considering its region

Tests

  • All tests have been updated to reflect dictionary-based return values
  • All existing and new tests pass

Xiaoyun Liu added 30 commits February 19, 2026 16:16
Copy link
Copy Markdown

@mihailefter mihailefter left a comment

Choose a reason for hiding this comment

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

A few minor things to address, left as inline comments. Could you also update the Python version matrix in .github/workflows/python-package.yml to "3.10""3.13"? The current versions are all EOL and the CI failures on 3.6/3.7 seem to be a direct consequence of the new type hints introduced in this PR. Also, could you add the specific Python version classifiers to setup.cfg?


if degenerate:
if region == 'u':
pos_m["region"] = '-'
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Inconsistent quote style: pos_m["region"] uses double quotes, while we opted for single quotes.

return 0

def to_position(self, coordinate):
def to_position(self, coordinate: int) -> dict[str: int | str]:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Invalid type argument. It should be: dict[str, int | str].

class Locus(object):
"""Locus object."""
def __init__(self, location, inverted=False):
def __init__(self, location: list[tuple[int, int]], inverted=False) -> None:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is this type hint correct? location looks like it should be tuple[int, int] rather than list[tuple[int, int]].

"""Convert a protein position (p.) to a coordinate.

:arg tuple position: Protein position (p.).
:arg dict position: Protein position model(p.).
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Docstring argument name mismatch: :arg dict position: should be :arg dict pos_m:.

Comment on lines +63 to +65
multilocus_pos_m = {**pos_m}
region = multilocus_pos_m['region']
multilocus_pos_m['position'] = multilocus_pos_m['position'] - 1
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could be collapsed into one line, as already done in multi_locus.py:98: multilocus_pos_m = {**pos_m, 'position': ...?

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