Skip to content

Replace Iterable with Collection/Sequence, remove stale type: ignore - #576

Open
ecomodeller wants to merge 2 commits into
mainfrom
refactor/replace-iterable-type-hints
Open

Replace Iterable with Collection/Sequence, remove stale type: ignore#576
ecomodeller wants to merge 2 commits into
mainfrom
refactor/replace-iterable-type-hints

Conversation

@ecomodeller

@ecomodeller ecomodeller commented Feb 12, 2026

Copy link
Copy Markdown
Member

Summary

  • Replace Iterable type hints with Collection (unordered: by, mod_items, comparers) and Sequence (ordered: metrics, obs/mod lists) for more precise API contracts
  • Remove 49 unused # type: ignore comments confirmed stale by mypy --warn-unused-ignores

Why

Iterable accepts single-pass iterators (generators) which would silently break code that iterates multiple times (e.g. ComparerCollection.__init__). Collection and Sequence better express the actual requirements and help downstream users relying on static type checking. Removing stale type: ignore comments ensures future type errors aren't silently suppressed.

…ore comments

Use Collection for unordered containers (by, mod_items, aux_items, comparers)
and Sequence for ordered ones (metrics, obs/mod lists). Remove 49 unused
type: ignore comments confirmed stale by mypy --warn-unused-ignores.
@ecomodeller ecomodeller added the typing Type hints, to be checked by e.g. mypy label Aug 27, 2026
@ecomodeller

Copy link
Copy Markdown
Member Author

@copilot resolve the merge conflicts in this pull request

Co-authored-by: ecomodeller <614215+ecomodeller@users.noreply.github.com>

Copilot AI commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved by merging origin/main and fixing the conflicts in the affected files. Included in commit f8c760c.

from __future__ import annotations
from typing import Callable, Iterable, List, Tuple, Union
from collections.abc import Collection
from typing import Callable, Optional, List, Tuple, Union

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

How come this wasn't caught by ruff?


@overload
def __getitem__(self, x: slice | Iterable[Hashable]) -> ComparerCollection: ... # type: ignore[overload-overlap]
def __getitem__(self, x: slice | Collection[Hashable]) -> ComparerCollection: ...

@overload
def __getitem__(self, key: Iterable[Hashable]) -> SkillGrid: ...
def __getitem__(self, key: Collection[Hashable]) -> SkillGrid: ...
Comment thread src/modelskill/skill.py

@overload
def __getitem__(self, key: Iterable[Hashable]) -> SkillTable: ...
def __getitem__(self, key: Collection[Hashable]) -> SkillTable: ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

typing Type hints, to be checked by e.g. mypy

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants