Conversation
…write degenerate in the same flow.
… into tuple_to_dataclass
mihailefter
left a comment
There was a problem hiding this comment.
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"] = '-' |
There was a problem hiding this comment.
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]: |
There was a problem hiding this comment.
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: |
There was a problem hiding this comment.
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.). |
There was a problem hiding this comment.
Docstring argument name mismatch: :arg dict position: should be :arg dict pos_m:.
| multilocus_pos_m = {**pos_m} | ||
| region = multilocus_pos_m['region'] | ||
| multilocus_pos_m['position'] = multilocus_pos_m['position'] - 1 |
There was a problem hiding this comment.
Could be collapsed into one line, as already done in multi_locus.py:98: multilocus_pos_m = {**pos_m, 'position': ...?
Pull Request Details
This pull request replaces tuple-based return values with dictionaries across the crossmapper. This change introduces dictionaries with explicit keys, improving:
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:
positionvalues without considering its regionTests