Code Health Enhancements on Core APIs

[albertomontesg]

Background

First of all I would like to provide some personal pointers from my side. I have almost 10 years of experience working with Python and on some codebases that are of a size comparable to Home Assistant, at very big companies, and there are some lessons that I have learned that I believe could be ported to Home Assistant in order to make it better at the maintenance level and developer experience.

I am a strong believer that good APIs (how we define the interfaces of the library) can make a huge difference on how developers write code, on how easy is to understand the code by newcomers and how error prone can be implementations done by external developers. Having good APIs is very hard and difficult to get it right at first, but investing to improve them can make a difference.

Finally I would like to highlight that any point raised here is not a critic on the current way of doing things at Home Assistant. I acknowledge (and experienced myself), how scaling a project and doing things the right way can be really hard, and considering the time HA started, many changes have been done in Python that helps to improve the developer life, but it was missing many years ago so it is more than understandable the patterns we currently see in HA codebase.

High Level Overview

One of the best things that struck me when I wanted to contribute to HA was the large amount (even abuse) of dictionaries as data structures with constant variables to define the keys. The shared const.py file has ~1k constant variables and this does not count the const variables defined at the different platforms and entities. Every piece of data that is being transferred in HA is by default represented by dictionaries which are not type checker friendly, are fully dynamic and can be error prone when some integrations populate the data at an expected schema.

Python allows this given its dynamicness of the type system, but with more recent introductions of type checking tooling, is it possible to get a more robust codebase that relies more on defined classes.

Readability and Code Health Enhancements

This entry proposes some changes that could help the readability and improvement of the overall code health with the goal to guide the users to write better and safer code by providing the best interfaces that guide you thought the best practices

Dataclasses instead of free form dictionaries

There are some problems with using dict as the standard struct type:

• Overload of constant variables
• Not easy to find the schema of specific structures
• TypedDict (used on Event class) provides a good constructor interface, but still when accessing the attributes it requires having the keys.
• Using dictionaries, even though most implementations rely on shared constants, this is not forcing the user to use it which can be error prone

The proposal could be to rely on dataclasses to construct the structs that need to be passed around the HA system. Some of the benefits on this approach are:

• Instances can be frozen (so can not be mutated). This can remove the need to use ReadOnlyDict as we can pass around dataclasses until the point where we need to send the data through the wired by encoding it as a json.
• Dataclasses provide a good schema reference as a single class contains the expected fields and their types
• Dataclasses can be inherited (therefore extended) by specific platforms that provide more data needed (similar to what Event already does)
• Dataclasses can be encoded to a dictionary (including child members) which would happen before sending it through the wire (as json encoding)

Some examples of possible candidates of classes or structs in core.py that could be defined as dataclasses could be (this list is not exhaustive):

• Event
• Context
• State
• The attributes of a State
• ServiceCall
• Config

On this dataclasses approach, something that could also be enforced are:

1. The structure is frozen. Only in cases where updating it is necessary, we can create new instances with dataclasses.replace(...). This way we can ensure immutability on the consumer of those structures.
2. Setting them by default as kwonly so all arguments are passed as keyword arguments and not as positional arguments.

Do not abuse of **kwargs in methods signatures.

On many interfaces required for the users to implement their components, we can see how many methods are marked with a single inputs being **kwargs:

• [ClimateEntity] set_temperature: link
• [WaterHeaterEntity] set_temperature: link
• [CoverEntity]: almost all methods even though most of them take no arguments or a single one.

A solution to this would be to redefine the signature to marked the arguments explicit. This change should be backwards compatible with custom components in HACS given that a child can have a **kwargs on the signature while the parent has the proper arguments defined.

A draft with some of this changes also done on all components can be found here () which shows how updating the signature to explicit arguments and types, can significantly simplify the logic that the users define.

More keyword-only arguments

reference

Too many methods with many arguments that are passed as positional arguments which can be hard to read and is more error prone. Examples: 1, 2, 3

A possible solution would be to convert signatures with more than 3 arguments to make them keyword args only (through the * operator).

Type aliasing of common keys

It is common to see that there are many keys and identifiers that are represented as mare strings. On type annotation, using just str does not provide you with full guarantees as a different identifier could be passed there.

Example:

class Event:
  id: str

class Entity:
  id: str

def lookup_entity(id: str):
  ...

# you can pass any string to that method without anyone complaining (linter or type checker)
lookup_entity(Event.id)  # not what we want but no checker will fail.

Counter example that provides better checks by the type checker:

EventId: TypeAlias = str
EntityId: TypeAlias = str

class Event:
  id: EventId

class Entity:
  id: EntityId

def lookup_entity(id: EntityId):
  ...

# mypy will raise an error on this line as not the correct type is being passed.
lookup_entity(Event.context_id)

Longer Term

A long term approach would be to document best practices when it comes to coding and extend the core APIs by following best practices that could reduce long term maintenance burden, cognitive load on readers, and remove error prone patterns. This best practices can be enforced at review time and referenced when providing feedback on future changes.

Possible Migration

Some of the changes described here are not very straighforward and would have to be done in a smooth manner. In some cases, some classes could be moved as dataclasses with very minimal changes across HA codebase (example for Context).

On others (for example making a stronger type the payload of the Event data, or the attributes of the entity State), there should be some transition process where the existing Mapping[str, Any] and some struct type could coexists. One approach to do so could be the following:

# Then we can define an abstract base class for all structures.
class Struct(abc.ABC):

  def __init_subclass__(cls):
    cls = dataclasses.dataclass(cls, kwonly=True, frozen=True)
    # And any other customization needed.
    return cls

  def as_dict(self) -> dict[str, Any]:
    ...

# We define the generic data type to be defined on an Event.
_DataT = TypeVar("_DataT", Struct, Mapping[str, Any], default=Mapping[str, Any])

# We update the signature of the classes storing that payload:
class Event(Generic[_DataT]):
  ...
  data: _DataT | None = None

  # Independently of the class passed to the Event as a payload we can serialize it.
  def as_dict(self):
    return {
      ...
      'data': self.data.as_dict() if isinstance(self.data, Struct) else self.data,
    }

Soliciting Feedback

I would like to hear from core contributors about your thoughts on this proposal and more in detail about the following questions:

• Does the problems and challenges I have highlighted are also perceived by others?
• Does the possible ideas make sense or are too unrealistic? Is it something worth investing one?
• Is there room to make readability and code quality recommendations at large scale moving forward to push for the right direction?