Use better return types

[cdce8p]

Small improvement. Change return types from Iterable to list. Usually it's recommend that the return type be as specific as possible since that helps with downstream typing.

Came across this issue while testing a new mypy feature with Home Assistant.

[allenporter]

Thanks. So my impression is that it if something takes an input then its still better to accept an iterable input of possible rather than a list, but for a return value being more specific/accurate is better (e.g. so you can use properties of the list)? I think i get it, thanks.

[allenporter]

As is, this reserves the right to return a more efficient return value in the future (e.g. an iterator that streams the results back, for example, rather than a list). Can you help me reason about why this is better? Did this actually fail a mypy check or do you have another specific goal in mind? Also happy if you have something to point me at with guidance on this.

[cdce8p]

So my impression is that it if something takes an input then its still better to accept an iterable input of possible rather than a list, but for a return value being more specific/accurate is better (e.g. so you can use properties of the list)?

Exactly. Input as broad as possible. I.e. Iterable allows the caller to pass any of list, tuple, set, ... . Same but in reverse for the return value -> as specific as possible. If the return type is list, it can be passed around to any function that accepts an Iterable or any abstract subclass (Collection, Sequence, ...). However an Iterable return value can only be passed to functions which also accept Iterable even though a list might be returned.

As is, this reserves the right to return a more efficient return value in the future (e.g. an iterator that streams the results back, for example, rather than a list).

Not exactly. I'd still consider changing the actual return value to be a breaking change (even if the typing would have allowed it). Typing is still optional after all. My general suggestion regarding typing is to type what you have and not what could be. The later would make it much more difficult for downstream consumers to deal with a library.

Can you help me reason about why this is better? Did this actually fail a mypy check or do you have another specific goal in mind? Also happy if you have something to point me at with guidance on this.

At the moment Home Assistant implicitly relies on the fact that no generator is returned. Especially with empty generators, the following checks would fail.

async def _async_get_recent_event_id(
    device_id: MediaId, device: Device
) -> MediaId | None:
    """Return thumbnail for most recent device event."""
    if CameraClipPreviewTrait.NAME in device.traits:
        clips = await device.event_media_manager.async_clip_preview_sessions()
        if not clips:  # <-- always false with Generator
            return None
        return MediaId(device_id.device_id, next(iter(clips)).event_token)
    images = await device.event_media_manager.async_image_sessions()
    if not images:  # <-- always false with Generator
        return None
    return MediaId(device_id.device_id, next(iter(images)).event_token)

https://github.com/home-assistant/core/blob/2022.10.0b0/homeassistant/components/nest/media_source.py#L462-L468

At the moment, I'm working on a mypy check to help detect these cases, of Iterables which can always be true, better. That's how I came across it initially. python/mypy#13762

[allenporter]

Thank you for the explanation! Very helpful.

[allenporter]

Let me know if there is any time frame you want this in HA and I can facilitate.

[cdce8p]

Let me know if there is any time frame you want this in HA and I can facilitate.

There isn't any rush. The check hasn't been accepted into mypy yet and the next release is at least one month out. No need to do an extra release just for it.

[cdce8p]

@allenporter Sorry do bug you. The mypy PR was merged today and will be included in the next release. Just wanted to ask if you have an idea when you want to do the release. It would be awesome if it would be fixed by the time I prepare the next mypy update for Home Assistant.

[allenporter]

Thanks for the reminder: home-assistant/core#82066

[cdce8p]

Thanks for the reminder: home-assistant/core#82066

Awesome! Thank you 👍🏻