Skip to content

chore(types): Phase 2 of #415, type config and mail_config - #452

Merged
rosspeili merged 3 commits into
ARPAHLS:mainfrom
UroojFatima-052:chore/issue-415-phase2-mypy
Oct 23, 2026
Merged

rosspeili merged 3 commits into
ARPAHLS:mainfrom
UroojFatima-052:chore/issue-415-phase2-mypy

Conversation

@UroojFatima-052

Copy link
Copy Markdown
Contributor

Refs #415

Phase 2: removes skillware.core.config and skillware.core.mail_config from [[tool.mypy.overrides]] and fixes the 4 errors that surfaced. Typing only, no behavior change.

Changes

  • mail_config.py (validate_addressbook_data): if has_email and isinstance(emails, list):. has_email already checks the type, but mypy can't narrow through a separate bool, so the check is repeated inline. Same runtime behavior.
  • mail_config.py (clear_mail_signature_settings): document = dict(_read_yaml(target)). _read_yaml returns a read-only Mapping, but the function edits it (pop, item assignment). Same pattern save_global_presentation_theme already uses.
  • config.py (_merge_layers): pass a list to merge_chain_layers instead of a generator, to match its Sequence signature. I fixed the call site rather than widening merge_chain_layers itself, since chains_config.py is outside this phase.
  • pyproject.toml: both modules dropped from the overrides list.
  • CHANGELOG.md: Unreleased entry.

Verification

  • mypy skillware: Success, no issues in 24 files
  • black --check / flake8 on both files: clean
  • pytest tests: 588 passed. The only failures are semantic_web_proxy, which fails locally on a missing lxml_html_clean package and is unrelated to this change.

Remaining overrides after this: skillware.cli and skillware.core.loader (Phase 3 onwards).

@rosspeili rosspeili left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM @UroojFatima-052, merging this one as is. Thanks for this. <3

@rosspeili
rosspeili merged commit d48d59a into ARPAHLS:main Oct 23, 2026
7 checks passed
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