jmcollin78 / versatile_thermostat

A full featured Thermostat for Home Assistant: presets, window, motion, presence and overpowering management
MIT License
327 stars 34 forks source link

Add more type hints in the thermostat classes and selected files #364

Closed pdcastro closed 10 months ago

pdcastro commented 10 months ago

This PR adds more type hints to selected files, mainly the thermostats classes. It started with wanting to know the type of the config_entry and entry_infos variables in the code, as I wondered if it was an object that had the same get(key[, default]) method signature as the standard dict class. I found that yes, it does. Then I thought, “Why don’t I add the type hint then?” From there it snowballed a bit and became this PR, but I think I am stopping here (I am not planning further PRs related to typing).

I don’t see type hints as just “hints for the developer that are ignored by the Python interpreter”, I see them as enabling the use of type checking tools such as pyright that help to avoid and catch bugs. Running pyright in the dev container (installed with pip install pyright) reported an increase from 379 to 412 errors before and after this PR respectively. The increased number errors is a good thing in the sense that the additional errors were simply being masked by the lack of type information. Pyright does not report an error if anything is assigned to a variable of type Unknown or Any, whereas it is an error to assign, say, a str value to a variable of type float | None for example.

Just glancing over the errors reported by pyright, I found at least one that points to code that cannot be right:

thermostat_climate.py:785:26 - error: Cannot access member "hvac_modes" for type "type[super]”

It is a line that reads super.hvac_modes instead of super().hvac_modes.

However, to confirm, this PR does not make any changes to functionality, nor fixes any issues. It just adds type hints. All tests pass with pytest and the code was formatted with black in the dev container.

jmcollin78 commented 10 months ago

Pushing to github pages failed but this is useless so I will merge.