Skip to content

MNT: Update type hints to modern Python 3.10+ syntax#511

Merged
adrinjalali merged 4 commits intomainfrom
copilot/update-typehints-python
Apr 13, 2026
Merged

MNT: Update type hints to modern Python 3.10+ syntax#511
adrinjalali merged 4 commits intomainfrom
copilot/update-typehints-python

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 13, 2026

Completes the modernization of type hints started in #426, replacing legacy typing module constructs with built-in equivalents throughout the codebase.

Changes

  • Optional[X]X | None across all files
  • Union[X, Y]X | Y where applicable
  • List[X]list[X], Dict[K, V]dict[K, V], Sequence[X]list[X]
  • Removed unused imports from typing (Optional, List, Sequence, Union, Dict) in 14 files
  • Fixed a bug from MNT Improve typehints. #426 in _visualize.py where trusted was annotated as [str] | None (bare list literal) instead of list[str] | None

Type distinction in trusted parameter

Nodes in _general.py / _audit.py that internally pass type objects (e.g. dict, list, set) to _get_trusted use list[str | type[Any]] | None, while nodes in _numpy.py, _sklearn.py, etc. that only deal with string type names use list[str] | None.

Before / After

# Before
from typing import Any, Iterator, List, Literal, Optional, Sequence

def delete(self, key: str | Sequence[str]) -> None: ...
trusted: Optional[List[str]] = None

# After
from typing import Any, Iterator, Literal

def delete(self, key: str | list[str]) -> None: ...
trusted: list[str] | None = None

Agent-Logs-Url: https://github.com/skops-dev/skops/sessions/6f0f4f08-6cde-47f1-81e0-33fd192d422e

Co-authored-by: adrinjalali <1663864+adrinjalali@users.noreply.github.com>
@adrinjalali
Copy link
Copy Markdown
Member

@copilot CI is failing, please try to fix.

Agent-Logs-Url: https://github.com/skops-dev/skops/sessions/b34daa0d-bc42-4654-b1c4-f2e0ee6d5d85

Co-authored-by: adrinjalali <1663864+adrinjalali@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 13, 2026

@copilot CI is failing, please try to fix.

Fixed in 0ad3c09. The pre-commit ruff hook detected a missing blank line between stdlib imports (import pickle) and local imports (from skops.cli._utils) in _convert.py — caused by removing the from typing import Optional line without preserving the import group separator. Added the blank line back.

@adrinjalali
Copy link
Copy Markdown
Member

@copilot please fix CI issues, until they're fixed.

…pixi.lock

Agent-Logs-Url: https://github.com/skops-dev/skops/sessions/563cf84e-e5a0-4b90-884c-d94804b31bb9

Co-authored-by: adrinjalali <1663864+adrinjalali@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 13, 2026

@copilot please fix CI issues, until they're fixed.

Fixed in a141218. Three issues were causing CI failures:

  1. ruff F821: memoize: bool = True was accidentally removed from Node.__init__ — restored it.
  2. mypy type errors: list[str | type[Any]] | None for trusted was incompatible with subclass list[str] | None values due to list invariance — changed to Sequence[str] | None in Node.__init__, CachedNode.__init__, get_tree, and _get_trusted; also fixed _general.py, _scipy.py, old/_general_v0.py, and _persist.py.
  3. pixi.lock format: pixi v0.39.5 in CI normalizes the lock file (./ → ., removes options: pypi-prerelease-mode blocks, normalizes sys_platform strings) — applied those changes directly.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • pixi.sh
    • Triggering command: /usr/bin/curl curl -fsSL REDACTED (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@adrinjalali
Copy link
Copy Markdown
Member

@copilot CI still fails.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 13, 2026

@copilot CI still fails.

Fixed in f60698b. The remaining failure was a mypy error in _markup.py:269: dict[str, Sequence[Any]] is not compatible with TableSection's expected Mapping[str, list[Any]] because tuples from zip(*body) are Sequence but not list. Fixed by explicitly converting to lists: {key: list(val) for key, val in zip(columns, data_transposed)} and updating the annotation to Mapping[str, list[Any]].

@adrinjalali adrinjalali marked this pull request as ready for review April 13, 2026 17:14
@adrinjalali adrinjalali merged commit 2c42dbb into main Apr 13, 2026
51 checks passed
@adrinjalali adrinjalali deleted the copilot/update-typehints-python branch April 13, 2026 17:35
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