ronantakizawa/codereview-bench huggingface dataset:
Repository: home-assistant/core
File: homeassistant/components/tado/coordinator.py (Python)

Code before:

from PyTado.interface import Tado
from requests import RequestException

from homeassistant.components.climate import PRESET_AWAY, PRESET_HOME
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed

if TYPE_CHECKING:
    from . import TadoConfigEntry

from .const import (
    CONF_FALLBACK,
    CONF_REFRESH_TOKEN,
    CONST_OVERLAY_TADO_DEFAULT,
    DOMAIN,
    INSIDE_TEMPERATURE_MEASUREMENT,
    PRESET_AUTO,
    TEMP_OFFSET,
)

_LOGGER = logging.getLogger(__name__)

MIN_TIME_BETWEEN_UPDATES = timedelta(minutes=4)
SCAN_INTERVAL = timedelta(minutes=5)


class TadoDataUpdateCoordinator(DataUpdateCoordinator[dict[str, dict]]):
    """Class to manage API calls from and to Tado via PyTado."""

    tado: Tado
    home_id: int
    home_name: str
    config_entry: TadoConfigEntry

    def __init__(
        self,
        hass: HomeAssistant,
        config_entry: TadoConfigEntry,
        tado: Tado,
        debug: bool = False,
    ) -> None:
        """Initialize the Tado data update coordinator."""
        super().__init__(
            hass,
            _LOGGER,
            config_entry=config_entry,
            name=DOMAIN,
            update_interval=SCAN_INTERVAL,
        )

Review comment (suggestion):
There's one in `__init__.py` as well:

https://github.com/erwindouna/ha-core/blob/3e1225580aa4643c50354ffffec5c91a132dbe32/homeassistant/components/tado/__init__.py#L51

Code after:

from PyTado.interface import Tado
from requests import RequestException

from homeassistant.components.climate import PRESET_AWAY, PRESET_HOME
from homeassistant.config_entries import ConfigEntry
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed

from .const import (
    CONF_FALLBACK,
    CONF_REFRESH_TOKEN,
    CONST_OVERLAY_TADO_DEFAULT,
    DOMAIN,
    INSIDE_TEMPERATURE_MEASUREMENT,
    PRESET_AUTO,
    TEMP_OFFSET,
)

_LOGGER = logging.getLogger(__name__)

MIN_TIME_BETWEEN_UPDATES = timedelta(minutes=4)
SCAN_INTERVAL = timedelta(minutes=5)

type TadoConfigEntry = ConfigEntry[TadoDataUpdateCoordinator]


class TadoDataUpdateCoordinator(DataUpdateCoordinator[dict[str, dict]]):
    """Class to manage API calls from and to Tado via PyTado."""

    tado: Tado
    home_id: int
    home_name: str
    config_entry: TadoConfigEntry

    def __init__(
        self,
        hass: HomeAssistant,
        config_entry: TadoConfigEntry,
        tado: Tado,
        debug: bool = False,
    ) -> None:
        """Initialize the Tado data update coordinator."""
        super().__init__(
            hass,
            _LOGGER,
            config_entry=config_entry,
            name=DOMAIN,
            update_interval=SCAN_INTERVAL,
        )
Repository: home-assistant/core
File: tests/components/conftest.py (Python)

Code before:
            _service_registry_async_call,
        ),
        patch(
            "homeassistant.core.ServiceRegistry.async_register",
            _service_registry_async_register,
        ),
    ):
        yield

    await asyncio.gather(*translation_coros)

    # Run final checks
    unused_ignore = [k for k, v in translation_errors.items() if v == "unused"]
    if unused_ignore:
        # Some ignored translations were not used
        pytest.fail(
            f"Unused ignore translations: {', '.join(unused_ignore)}. "
            "Please remove them from the ignore_missing_translations fixture."
        )
    for description in translation_errors.values():
        if description != "used":
            pytest.fail(description)


@pytest.fixture(name="enable_experimental_triggers_conditions")
def enable_experimental_triggers_conditions() -> Generator[None]:
    """Enable experimental triggers and conditions."""
    with patch(
        "homeassistant.components.labs.async_is_preview_feature_enabled",
        return_value=True,
    ):
        yield


Review comment (suggestion):
maybe later we should rename this to `enable_labs_preview_features`, since that is what this does.

Code after:
            _service_registry_async_call,
        ),
        patch(
            "homeassistant.core.ServiceRegistry.async_register",
            _service_registry_async_register,
        ),
    ):
        yield

    await asyncio.gather(*translation_coros)

    # Run final checks
    unused_ignore = [k for k, v in translation_errors.items() if v == "unused"]
    if unused_ignore:
        # Some ignored translations were not used
        pytest.fail(
            f"Unused ignore translations: {', '.join(unused_ignore)}. "
            "Please remove them from the ignore_missing_translations fixture."
        )
    for description in translation_errors.values():
        if description != "used":
            pytest.fail(description)


@pytest.fixture(name="enable_labs_preview_features")
def enable_labs_preview_features() -> Generator[None]:
    """Enable labs preview features."""
    with patch(
        "homeassistant.components.labs.async_is_preview_feature_enabled",
        return_value=True,
    ):
        yield
Repository: home-assistant/core
File: homeassistant/components/mfi/sensor.py (Python)

Code before:
    """Set up mFi sensors."""
    host: str = config[CONF_HOST]
    username: str = config[CONF_USERNAME]
    password: str = config[CONF_PASSWORD]
    use_tls: bool = config[CONF_SSL]
    verify_tls: bool = config[CONF_VERIFY_SSL]
    default_port = 6443 if use_tls else 6080
    network_port: int = config.get(CONF_PORT, default_port)

    try:
        client = MFiClient(
            host,
            username,
            password,
            port=network_port,
            use_tls=use_tls,
            verify=verify_tls,
        )
    except (FailedToLogin, requests.exceptions.ConnectionError) as ex:
        _LOGGER.error("Unable to connect to mFi: %s", str(ex))
        return

    add_entities(
        MfiSensor(port)
        for device in cast(list[MFiDevice], client.get_devices())
        for port in cast(dict[str, MFiPort], device.ports).values()
        if port.model in SENSOR_MODELS
    )


class MfiSensor(SensorEntity):
    """Representation of a mFi sensor."""

    def __init__(self, port: MFiPort) -> None:
        """Initialize the sensor."""
        self._port = port

    @property
    def name(self) -> str:
        """Return the name of the sensor."""
        return self._port.label

    @property
    def native_value(self) -> StateType:
        """Return the state of the sensor."""
        try:
            tag = self._port.tag
        except ValueError:
            tag = None
        if tag is None:
            return STATE_OFF

Review comment (suggestion):
Meh, I think casting everything that the device returns is a bad practice, instead the library should be py.typed

Code after:
    default_port = 6443 if use_tls else 6080
    network_port: int = config.get(CONF_PORT, default_port)

    try:
        client = MFiClient(
            host,
            username,
            password,
            port=network_port,
            use_tls=use_tls,
            verify=verify_tls,
        )
    except (FailedToLogin, requests.exceptions.ConnectionError) as ex:
        _LOGGER.error("Unable to connect to mFi: %s", str(ex))
        return

    add_entities(
        MfiSensor(port)
        for device in client.get_devices()
        for port in device.ports.values()
        if port.model in SENSOR_MODELS
    )


class MfiSensor(SensorEntity):
    """Representation of a mFi sensor."""

    def __init__(self, port: MFiPort) -> None:
        """Initialize the sensor."""
        self._port = port

    @property
    def name(self) -> str:
        """Return the name of the sensor."""
        return self._port.label

    @property
    def native_value(self) -> StateType:
        """Return the state of the sensor."""
        try:
            tag = self._port.tag
        except ValueError:
            tag = None
        if tag is None:
            return STATE_OFF
        if self._port.model == "Input Digital":
            return STATE_ON if self._port.value > 0 else STATE_OFF
        digits = DIGITS.get(self._port.tag, 0)
        return round(self._port.value, digits)

    @property
Repository: home-assistant/core
File: homeassistant/components/utility_meter/const.py (Python)

Code before:

from homeassistant.util.hass_dict import HassKey

DOMAIN = "utility_meter"

QUARTER_HOURLY = "quarter-hourly"
HOURLY = "hourly"
DAILY = "daily"
WEEKLY = "weekly"
MONTHLY = "monthly"
BIMONTHLY = "bimonthly"
QUARTERLY = "quarterly"
YEARLY = "yearly"

METER_TYPES = [
    QUARTER_HOURLY,
    HOURLY,
    DAILY,
    WEEKLY,
    MONTHLY,
    BIMONTHLY,
    QUARTERLY,
    YEARLY,
]

DATA_UTILITY: HassKey[dict[str, dict[str, Any]]] = HassKey(DOMAIN)
DATA_TARIFF_SENSORS = "utility_meter_sensors"

CONF_METER = "meter"
CONF_SOURCE_SENSOR = "source"
CONF_METER_TYPE = "cycle"
CONF_METER_OFFSET = "offset"
CONF_METER_DELTA_VALUES = "delta_values"
CONF_METER_NET_CONSUMPTION = "net_consumption"
CONF_METER_PERIODICALLY_RESETTING = "periodically_resetting"
CONF_PAUSED = "paused"
CONF_TARIFFS = "tariffs"
CONF_TARIFF = "tariff"
CONF_TARIFF_ENTITY = "tariff_entity"
CONF_CRON_PATTERN = "cron"
CONF_SENSOR_ALWAYS_AVAILABLE = "always_available"

ATTR_TARIFF = "tariff"
ATTR_TARIFFS = "tariffs"
ATTR_VALUE = "value"
ATTR_CRON_PATTERN = "cron pattern"
ATTR_NEXT_RESET = "next_reset"

SIGNAL_START_PAUSE_METER = "utility_meter_start_pause"
SIGNAL_RESET_METER = "utility_meter_reset"


Review comment (refactor):
Small change here: we now store the data inside `hass.data["utility_meter"]` not against `hass.data["utility_meter_data"]`

Code after:

from datetime import timedelta
from typing import TYPE_CHECKING, Final, TypedDict

from homeassistant.util.hass_dict import HassKey

if TYPE_CHECKING:
    from .sensor import UtilityMeterSensor

DOMAIN = "utility_meter"

QUARTER_HOURLY = "quarter-hourly"
HOURLY = "hourly"
DAILY = "daily"
WEEKLY = "weekly"
MONTHLY = "monthly"
BIMONTHLY = "bimonthly"
QUARTERLY = "quarterly"
YEARLY = "yearly"

METER_TYPES = [
    QUARTER_HOURLY,
    HOURLY,
    DAILY,
    WEEKLY,
    MONTHLY,
    BIMONTHLY,
    QUARTERLY,
    YEARLY,
]

DATA_UTILITY: HassKey[dict[str, MeterInformation]] = HassKey(DOMAIN)
DATA_TARIFF_SENSORS: Final = "utility_meter_sensors"

CONF_METER = "meter"
CONF_SOURCE_SENSOR: Final = "source"
CONF_METER_TYPE = "cycle"
CONF_METER_OFFSET: Final = "offset"
CONF_METER_DELTA_VALUES: Final = "delta_values"
CONF_METER_NET_CONSUMPTION: Final = "net_consumption"
CONF_METER_PERIODICALLY_RESETTING: Final = "periodically_resetting"
CONF_PAUSED = "paused"
CONF_TARIFFS = "tariffs"
CONF_TARIFF = "tariff"
CONF_TARIFF_ENTITY: Final = "tariff_entity"
CONF_CRON_PATTERN = "cron"
CONF_SENSOR_ALWAYS_AVAILABLE: Final = "always_available"

ATTR_TARIFF = "tariff"
ATTR_TARIFFS = "tariffs"
ATTR_VALUE = "value"
Repository: home-assistant/core
File: homeassistant/helpers/condition.py (Python)

Code before:

    return referenced


@callback
def async_extract_areas(config: ConfigType | Template) -> set[str]:
    """Extract areas from a condition."""
    return _async_extract_targets(config, ATTR_AREA_ID)


@callback
def async_extract_floors(config: ConfigType | Template) -> set[str]:
    """Extract floors from a condition."""
    return _async_extract_targets(config, ATTR_FLOOR_ID)


@callback
def async_extract_labels(config: ConfigType | Template) -> set[str]:
    """Extract labels from a condition."""
    return _async_extract_targets(config, ATTR_LABEL_ID)


@callback
def _async_extract_targets(
    config: ConfigType | Template,
    target: Literal["entity_id", "device_id", "area_id", "floor_id", "label_id"],
) -> set[str]:
    """Extract targets from a condition."""
    referenced: set[str] = set()
    to_process = deque([config])

    while to_process:
        config = to_process.popleft()
        if isinstance(config, Template):
            continue

        condition = config[CONF_CONDITION]

        if condition in ("and", "not", "or"):
            to_process.extend(config["conditions"])
            continue

        if target_labels := _get_targets_from_condition_config(config, target):
            referenced.update(target_labels)

    return referenced


@callback
def _get_targets_from_condition_config(
    config: ConfigType,

Review comment (suggestion):
I think the name of this parameter should make it clear it specifies a specific subtype/category of the target.
```suggestion
    target_type: Literal["entity_id", "device_id", "area_id", "floor_id", "label_id"],
```

Code after:

    return referenced


@callback
def async_extract_areas(config: ConfigType | Template) -> set[str]:
    """Extract areas from a condition."""
    return _async_extract_targets(config, ATTR_AREA_ID)


@callback
def async_extract_floors(config: ConfigType | Template) -> set[str]:
    """Extract floors from a condition."""
    return _async_extract_targets(config, ATTR_FLOOR_ID)


@callback
def async_extract_labels(config: ConfigType | Template) -> set[str]:
    """Extract labels from a condition."""
    return _async_extract_targets(config, ATTR_LABEL_ID)


@callback
def _async_extract_targets(
    config: ConfigType | Template,
    target_type: Literal["entity_id", "device_id", "area_id", "floor_id", "label_id"],
) -> set[str]:
    """Extract targets from a condition."""
    referenced: set[str] = set()
    to_process = deque([config])

    while to_process:
        config = to_process.popleft()
        if isinstance(config, Template):
            continue

        condition = config[CONF_CONDITION]

        if condition in ("and", "not", "or"):
            to_process.extend(config["conditions"])
            continue

        if targets := _get_targets_from_condition_config(config, target_type):
            referenced.update(targets)

    return referenced


@callback
def _get_targets_from_condition_config(
    config: ConfigType,
Repository: home-assistant/core
File: tests/components/siren/test_condition.py (Python)

Code before:
"""Test siren conditions."""

from typing import Any

import pytest

from homeassistant.const import STATE_OFF, STATE_ON
from homeassistant.core import HomeAssistant

from tests.components import (
    ConditionStateDescription,
    assert_condition_gated_by_labs_flag,
    create_target_condition,
    parametrize_condition_states,
    parametrize_target_entities,
    set_or_remove_state,
    target_entities,
)


@pytest.fixture(autouse=True, name="stub_blueprint_populate")
def stub_blueprint_populate_autouse(stub_blueprint_populate: None) -> None:
    """Stub copying the blueprints to the config folder."""


@pytest.fixture
async def target_sirens(hass: HomeAssistant) -> list[str]:
    """Create multiple siren entities associated with different targets."""
    return (await target_entities(hass, "siren"))["included"]


@pytest.mark.parametrize(
    "condition",
    [
        "siren.is_off",
        "siren.is_on",
    ],
)
async def test_siren_conditions_gated_by_labs_flag(
    hass: HomeAssistant, caplog: pytest.LogCaptureFixture, condition: str
) -> None:
    """Test the siren conditions are gated by the labs flag."""
    await assert_condition_gated_by_labs_flag(hass, caplog, condition)


@pytest.mark.usefixtures("enable_labs_preview_features")
@pytest.mark.parametrize(
    ("condition_target_config", "entity_id", "entities_in_target"),
    parametrize_target_entities("siren"),

Review comment (suggestion):
```suggestion
pytestmark = pytest.mark.usefixtures("stub_blueprint_populate")
```

In my opinion clearer as an empty fixture just to specify autouse

Code after:
"""Test siren conditions."""

from typing import Any

import pytest

from homeassistant.const import STATE_OFF, STATE_ON
from homeassistant.core import HomeAssistant

from tests.components import (
    ConditionStateDescription,
    assert_condition_gated_by_labs_flag,
    create_target_condition,
    parametrize_condition_states_all,
    parametrize_condition_states_any,
    parametrize_target_entities,
    set_or_remove_state,
    target_entities,
)


@pytest.fixture(autouse=True, name="stub_blueprint_populate")
def stub_blueprint_populate_autouse(stub_blueprint_populate: None) -> None:
    """Stub copying the blueprints to the config folder."""


@pytest.fixture
async def target_sirens(hass: HomeAssistant) -> list[str]:
    """Create multiple siren entities associated with different targets."""
    return (await target_entities(hass, "siren"))["included"]


@pytest.fixture
async def target_switches(hass: HomeAssistant) -> list[str]:
    """Create multiple switch entities associated with different targets."""
    return (await target_entities(hass, "switch"))["included"]


@pytest.mark.parametrize(
    "condition",
    [
        "siren.is_off",
        "siren.is_on",
    ],
)
async def test_siren_conditions_gated_by_labs_flag(
    hass: HomeAssistant, caplog: pytest.LogCaptureFixture, condition: str
) -> None:
    """Test the siren conditions are gated by the labs flag."""
Repository: home-assistant/core
File: tests/components/hassio/test_addon_panel.py (Python)

Code before:
            "result": "ok",
            "data": {
                "panels": {
                    "test_addon": {
                        "enable": True,
                        "title": "Test Addon",
                        "icon": "mdi:test-tube",
                        "admin": True,
                    },
                }
            },
        },
    )

    with patch(
        "homeassistant.components.hassio.addon_panel.frontend.async_register_built_in_panel"
    ) as mock_register:
        await async_setup_component(hass, "hassio", {})
        await hass.async_block_till_done()

        # Verify that async_register_built_in_panel was called with correct arguments
        # for our test addon
        mock_register.assert_any_call(
            hass,
            "app",
            frontend_url_path="app/test_addon",
            sidebar_title="Test Addon",
            sidebar_icon="mdi:test-tube",
            require_admin=True,
            config={"addon": "test_addon"},
        )


Review comment (suggestion):
```suggestion
            frontend_url_path="test_addon",
```

Code after:
            "result": "ok",
            "data": {
                "panels": {
                    "test_addon": {
                        "enable": True,
                        "title": "Test Addon",
                        "icon": "mdi:test-tube",
                        "admin": True,
                    },
                }
            },
        },
    )

    with patch(
        "homeassistant.components.hassio.addon_panel.frontend.async_register_built_in_panel"
    ) as mock_register:
        await async_setup_component(hass, "hassio", {})
        await hass.async_block_till_done()

        # Verify that async_register_built_in_panel was called with correct arguments
        # for our test addon
        mock_register.assert_any_call(
            hass,
            "app",
            frontend_url_path="test_addon",
            sidebar_title="Test Addon",
            sidebar_icon="mdi:test-tube",
            require_admin=True,
            config={"addon": "test_addon"},
        )
Repository: home-assistant/core
File: homeassistant/components/bang_olufsen/services.py (Python)

Code before:
"""Media player entity for the Bang & Olufsen integration."""

from __future__ import annotations

import voluptuous as vol

from homeassistant.components.media_player import DOMAIN as MEDIA_PLAYER_DOMAIN
from homeassistant.core import HomeAssistant, callback
from homeassistant.helpers import config_validation as cv, service

from .const import BEOLINK_JOIN_SOURCES, DOMAIN


@callback
def async_setup_services(hass: HomeAssistant) -> None:
    """Home Assistant services."""

    jid_regex = vol.Match(
        r"(^\d{4})[.](\d{7})[.](\d{8})(@products\.bang-olufsen\.com)$"
    )

    service.async_register_platform_entity_service(
        hass,
        DOMAIN,
        "beolink_join",
        entity_domain=MEDIA_PLAYER_DOMAIN,

Review comment (suggestion):
```suggestion
"""Services for Bang & Olufsen integration."""
```

Code after:
"""Services for Bang & Olufsen integration."""

from __future__ import annotations

import voluptuous as vol

from homeassistant.components.media_player import DOMAIN as MEDIA_PLAYER_DOMAIN
from homeassistant.core import HomeAssistant, callback
from homeassistant.helpers import config_validation as cv, service

from .const import BEOLINK_JOIN_SOURCES, DOMAIN


@callback
def async_setup_services(hass: HomeAssistant) -> None:
    """Home Assistant services."""

    jid_regex = vol.Match(
        r"(^\d{4})[.](\d{7})[.](\d{8})(@products\.bang-olufsen\.com)$"
    )

    service.async_register_platform_entity_service(
        hass,
        DOMAIN,
        "beolink_join",
        entity_domain=MEDIA_PLAYER_DOMAIN,
Repository: home-assistant/core
File: homeassistant/components/waterfurnace/__init__.py (Python)

Code before:
    )


async def async_setup_entry(
    hass: HomeAssistant, entry: WaterFurnaceConfigEntry
) -> bool:
    """Set up WaterFurnace from a config entry."""
    username = entry.data[CONF_USERNAME]
    password = entry.data[CONF_PASSWORD]

    client = WaterFurnace(username, password)

    try:
        await hass.async_add_executor_job(client.login)
    except WFCredentialError as err:
        raise ConfigEntryAuthFailed(
            "Authentication failed. Please update your credentials."
        ) from err

    if not client.gwid:
        raise ConfigEntryNotReady(
            "Failed to connect to WaterFurnace service: No GWID found for device"
        )

    coordinator = WaterFurnaceCoordinator(hass, client, entry)
    entry.runtime_data = coordinator
    await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)

    return True


Review comment (suggestion):
We should run `await coordinator.async_config_entry_first_refresh()` to make sure the coordinator has a state and is able to fetch the data needed

Code after:
    )


async def async_setup_entry(
    hass: HomeAssistant, entry: WaterFurnaceConfigEntry
) -> bool:
    """Set up WaterFurnace from a config entry."""
    username = entry.data[CONF_USERNAME]
    password = entry.data[CONF_PASSWORD]

    client = WaterFurnace(username, password)

    try:
        await hass.async_add_executor_job(client.login)
    except WFCredentialError as err:
        raise ConfigEntryAuthFailed(
            "Authentication failed. Please update your credentials."
        ) from err

    if not client.gwid:
        raise ConfigEntryNotReady(
            "Failed to connect to WaterFurnace service: No GWID found for device"
        )

    coordinator = WaterFurnaceCoordinator(hass, client, entry)
    entry.runtime_data = coordinator
    await coordinator.async_config_entry_first_refresh()
    await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)

    return True