Repository navigation
Conversation
Co-authored-by: Copilot <copilot@github.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The pre-commit hook weakens strict checking by ignoring missing imports and omitting the cryptography dependency.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Adds strict typing and PEP 561 metadata across the library while improving handling of incomplete responses.
Changes:
- Adds mypy tooling and package type metadata.
- Annotates the client, CLI, entities, and device types.
- Handles missing login/XML fields and tests missing challenges.
| File | Description |
|---|---|
.pre-commit-config.yaml |
Adds the mypy hook. |
requirements_dev.txt |
Adds mypy and request stubs. |
setup.cfg |
Configures strict mypy and typed package data. |
pyfritzhome/py.typed |
Marks the package as typed. |
pyfritzhome/cli.py |
Types CLI handlers and arguments. |
pyfritzhome/errors.py |
Types exceptions and adds login details. |
pyfritzhome/fritzhome.py |
Types the main client and hardens parsing. |
pyfritzhome/fritzhomedevice.py |
Types device construction and updates. |
pyfritzhome/devicetypes/fritzhomeentitybase.py |
Types shared entity state and helpers. |
pyfritzhome/devicetypes/fritzhomedevicebase.py |
Types base device attributes. |
pyfritzhome/devicetypes/fritzhomedevicealarm.py |
Types alarm devices. |
pyfritzhome/devicetypes/fritzhomedeviceblind.py |
Types blind devices. |
pyfritzhome/devicetypes/fritzhomedevicebutton.py |
Types buttons and XML helpers. |
pyfritzhome/devicetypes/fritzhomedevicehumidity.py |
Types humidity devices. |
pyfritzhome/devicetypes/fritzhomedevicelevel.py |
Types level controls. |
pyfritzhome/devicetypes/fritzhomedevicelightbulb.py |
Types color and light controls. |
pyfritzhome/devicetypes/fritzhomedevicepowermeter.py |
Types power measurements. |
pyfritzhome/devicetypes/fritzhomedevicerepeater.py |
Types repeater devices. |
pyfritzhome/devicetypes/fritzhomedeviceswitch.py |
Types switch devices. |
pyfritzhome/devicetypes/fritzhomedevicetemperature.py |
Types temperature devices. |
pyfritzhome/devicetypes/fritzhomedevicethermostat.py |
Types thermostat state and controls. |
pyfritzhome/devicetypes/fritzhometemplate.py |
Types and validates templates. |
pyfritzhome/devicetypes/fritzhometrigger.py |
Types trigger state. |
tests/test_fritzhome.py |
Tests missing login challenges. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
flabbamann
left a comment
There was a problem hiding this comment.
Hey @mib1185,
nice improvement, thank you 🎉👍.
I don't see any problems, the code changes are small and should make the code more robust.
But I would suggest to drop Python 3.9. It is EOL since almost a year and with 3.10 and up you can use the builtin union types like str | None instead of Optional[str]. I think it's the preferred way today. It's a bit easier to read and you don't need the imports. What do you think?
| groupinfo = node.find("groupinfo") | ||
| self.is_group = groupinfo is not None | ||
| if self.is_group: | ||
| if groupinfo is not None: |
There was a problem hiding this comment.
doesn't make a real difference, but I think I would revert this line 🙂.
There was a problem hiding this comment.
mypy has a different opinion about this 🙈 this is the mypy result with if self.is_group :
pyfritzhome/devicetypes/fritzhomedevicebase.py:59: error: Item "None" of "Optional[Element[str]]" has no attribute "findtext" [union-attr]
Found 1 error in 1 file (checked 1 source file)
There was a problem hiding this comment.
Ah, okay. Probably mypy can't be sure that self.is_group or groupinfo are not manipulated after assigning is_group. I did not think about that.
|
Hi @flabbamann |
Co-authored-by: Copilot <copilot@github.com>
|
Ah, nice. I didn't know that 👍 |

This adds mypy to the dev-requirements and also integrates it into the pre-commit configuration. Mypy is configured to only check library code, but not the tests. Mypy is used in strict mode. Finally we now distribute the package type information (according to pep 561).
Most of the adjustments were done on behalf of CoPilot, but checked in detail on my own.