Skip to content

Make model keyword-only in all Device subclass constructors - #2065

Merged
rytilahti merged 1 commit into
rytilahti:masterfrom
Acrobot:consolidate-constructors
Jun 27, 2026
Merged

Make model keyword-only in all Device subclass constructors#2065
rytilahti merged 1 commit into
rytilahti:masterfrom
Acrobot:consolidate-constructors

Conversation

@Acrobot

@Acrobot Acrobot commented Mar 27, 2026

Copy link
Copy Markdown
Contributor

The base Device.__init__ defines model as keyword-only (after *), but six subclasses had it as positional: FanP5, Fan1C, Huizuo, AirConditioningCompanion, AirConditioningCompanionMcn02, and Yeelight. This breaks Liskov substitution - code that works with the base class signature can't safely substitute these subclasses.

Fixes all six to use *, model: Optional[str] = ... matching the base class, and adds test_model_is_keyword_only that inspects every Device subclass signature to prevent regressions.

No positional model usage was found anywhere in the codebase, so this is a safe change.

Full test suite: 1345 passed (1286 on master + 59 new parametrized test_model_is_keyword_only cases). Same pre-existing failures only. Ruff clean.

Addresses #1156

The base Device.__init__ defines model as keyword-only (after *), but
six subclasses had it as positional: FanP5, Fan1C, Huizuo,
AirConditioningCompanion, AirConditioningCompanionMcn02, and Yeelight.

This fixes them all to match the base class and adds a
test_model_is_keyword_only test that inspects every Device subclass
signature to prevent regressions.

Addresses rytilahti#1156
@codecov

codecov Bot commented Mar 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.26%. Comparing base (c5cc0f2) to head (45cd99e).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2065   +/-   ##
=======================================
  Coverage   82.26%   82.26%           
=======================================
  Files         197      197           
  Lines       19145    19145           
  Branches     1052     1052           
=======================================
  Hits        15750    15750           
  Misses       3218     3218           
  Partials      177      177           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Acrobot

Acrobot commented Mar 27, 2026

Copy link
Copy Markdown
Contributor Author

These changes were generated using Claude Code under my supervision. I reviewed and approved all code before submitting. I'm not very familiar with this codebase, so apologies in advance for any mistakes.

@Acrobot
Acrobot marked this pull request as ready for review March 27, 2026 00:51
@Acrobot Acrobot mentioned this pull request Mar 27, 2026
22 tasks

@rytilahti rytilahti left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks good and straightforward to me, thank you @Acrobot!

@rytilahti
rytilahti merged commit 2b105fd into rytilahti:master Jun 27, 2026
22 of 24 checks passed
@Acrobot
Acrobot deleted the consolidate-constructors branch July 8, 2026 23:49
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