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

Code before:
from aiontfy.exceptions import (
    NtfyException,
    NtfyHTTPError,
    NtfyUnauthorizedAuthenticationError,
)
import voluptuous as vol
from yarl import URL

from homeassistant.components.notify import (
    ATTR_MESSAGE,
    ATTR_TITLE,
    NotifyEntity,
    NotifyEntityDescription,
    NotifyEntityFeature,
)
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import HomeAssistantError, ServiceValidationError
from homeassistant.helpers import config_validation as cv, entity_platform
from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback

from .const import DOMAIN
from .coordinator import NtfyConfigEntry
from .entity import NtfyBaseEntity

PARALLEL_UPDATES = 0
MAX_ACTIONS_ALLOWED = 3

SERVICE_PUBLISH = "publish"
ATTR_ATTACH = "attach"
ATTR_CALL = "call"
ATTR_CLICK = "click"
ATTR_DELAY = "delay"
ATTR_EMAIL = "email"
ATTR_ICON = "icon"
ATTR_MARKDOWN = "markdown"
ATTR_PRIORITY = "priority"
ATTR_TAGS = "tags"
ATTR_ACTION = "action"
ATTR_VIEW = "view"
ATTR_BROADCAST = "broadcast"
ATTR_HTTP = "http"
ATTR_LABEL = "label"
ATTR_URL = "url"
ATTR_CLEAR = "clear"
ATTR_POSITION = "position"
ATTR_INTENT = "intent"
ATTR_EXTRAS = "extras"
ATTR_METHOD = "method"
ATTR_HEADERS = "headers"
ATTR_BODY = "body"
ACTIONS_MAP = {

Review comment (suggestion):
I'd suggest including a source for the specified limit, as the number 3 seems quite arbitrary otherwise:
```suggestion

# According to the official ntfy.sh documentation, at most three action buttons are allowed
MAX_ACTIONS_ALLOWED = 3
```

Code after:
from aiontfy import Message
from aiontfy.exceptions import (
    NtfyException,
    NtfyHTTPError,
    NtfyUnauthorizedAuthenticationError,
)

from homeassistant.components import camera, image
from homeassistant.components.media_source import async_resolve_media
from homeassistant.components.notify import (
    NotifyEntity,
    NotifyEntityDescription,
    NotifyEntityFeature,
)
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import HomeAssistantError, ServiceValidationError
from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback

from .const import DOMAIN
from .coordinator import NtfyConfigEntry
from .entity import NtfyBaseEntity
from .services import (
    ACTIONS_MAP,
    ATTR_ACTION,
    ATTR_ACTIONS,
    ATTR_ATTACH_FILE,
    ATTR_FILENAME,
    ATTR_SEQUENCE_ID,
)

_LOGGER = logging.getLogger(__name__)

PARALLEL_UPDATES = 0


async def async_setup_entry(
    hass: HomeAssistant,
    config_entry: NtfyConfigEntry,
    async_add_entities: AddConfigEntryEntitiesCallback,
) -> None:
    """Set up the ntfy notification entity platform."""

    for subentry_id, subentry in config_entry.subentries.items():
        async_add_entities(
            [NtfyNotifyEntity(config_entry, subentry)], config_subentry_id=subentry_id
        )


class NtfyNotifyEntity(NtfyBaseEntity, NotifyEntity):
    """Representation of a ntfy notification entity."""
Repository: home-assistant/core
File: homeassistant/components/ntfy/notify.py (Python)

Code before:
"""ntfy notification entity."""

from __future__ import annotations

from datetime import timedelta
from typing import Any

import aiofiles
from aiontfy import Message
from aiontfy.exceptions import (
    NtfyException,
    NtfyHTTPError,
    NtfyUnauthorizedAuthenticationError,
)
import voluptuous as vol
from yarl import URL

from homeassistant.components import camera
from homeassistant.components.image import DATA_COMPONENT as IMAGE_DATA_COMPONENT
from homeassistant.components.media_source import async_resolve_media
from homeassistant.components.notify import (
    ATTR_MESSAGE,
    ATTR_TITLE,
    NotifyEntity,
    NotifyEntityDescription,
    NotifyEntityFeature,
)
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import HomeAssistantError, ServiceValidationError
from homeassistant.helpers import config_validation as cv, entity_platform
from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback
from homeassistant.helpers.selector import MediaSelector


Review comment (suggestion):
`aiofiles` is not a core dependency, thus meaning this becomes a requirement for this integration if this is imported (and thus should be added to the manifest).



Code after:
"""ntfy notification entity."""

from __future__ import annotations

from datetime import timedelta
import logging
from typing import Any

from aiontfy import Message
from aiontfy.exceptions import (
    NtfyException,
    NtfyHTTPError,
    NtfyUnauthorizedAuthenticationError,
)
import voluptuous as vol
from yarl import URL

from homeassistant.components import camera, image
from homeassistant.components.media_source import async_resolve_media
from homeassistant.components.notify import (
    ATTR_MESSAGE,
    ATTR_TITLE,
    NotifyEntity,
    NotifyEntityDescription,
    NotifyEntityFeature,
)
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import HomeAssistantError, ServiceValidationError
from homeassistant.helpers import config_validation as cv, entity_platform
from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback
from homeassistant.helpers.selector import MediaSelector

from .const import DOMAIN
Repository: home-assistant/core
File: homeassistant/components/yardian/coordinator.py (Python)

Code before:
from homeassistant.helpers.device_registry import DeviceInfo
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed

from .const import DOMAIN, MANUFACTURER

_LOGGER = logging.getLogger(__name__)

SCAN_INTERVAL = datetime.timedelta(seconds=30)


class YardianCombinedState:
    """Combined device state for Yardian."""

    def __init__(
        self,
        zones: list[list],
        active_zones: set[int],
        oper_info: OperationInfo,
    ) -> None:
        """Initialize combined state with zones, active_zones and oper_info."""
        self.zones = zones
        self.active_zones = active_zones
        self.oper_info = oper_info


class YardianUpdateCoordinator(DataUpdateCoordinator[YardianCombinedState]):
    """Coordinator for Yardian API calls."""

    config_entry: ConfigEntry

    def __init__(
        self,
        hass: HomeAssistant,
        entry: ConfigEntry,
        controller: AsyncYardianClient,
    ) -> None:
        """Initialize Yardian API communication."""
        super().__init__(
            hass,
            _LOGGER,
            config_entry=entry,
            name=entry.title,
            update_interval=SCAN_INTERVAL,
            always_update=False,
        )

        self.controller = controller
        self.yid = entry.data["yid"]
        self._name = entry.title
        self._model = entry.data["model"]
        self._serial = entry.data.get("serialNumber")

Review comment (suggestion):
personally I don't like `CombinedState` as a name, because it's unclear what the combination is

Code after:
from homeassistant.exceptions import ConfigEntryError
from homeassistant.helpers.device_registry import DeviceInfo
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed

from .const import DOMAIN, MANUFACTURER

_LOGGER = logging.getLogger(__name__)

SCAN_INTERVAL = datetime.timedelta(seconds=30)


@dataclass(slots=True)
class YardianZone:
    """Normalized metadata for a Yardian irrigation zone."""

    name: str
    is_enabled: bool


@dataclass
class YardianCoordinatorData:
    """Combined device state for Yardian."""

    zones: list[YardianZone]
    active_zones: set[int]
    oper_info: OperationInfo


class YardianUpdateCoordinator(DataUpdateCoordinator[YardianCoordinatorData]):
    """Coordinator for Yardian API calls."""

    config_entry: ConfigEntry

    def __init__(
        self,
        hass: HomeAssistant,
        entry: ConfigEntry,
        controller: AsyncYardianClient,
    ) -> None:
        """Initialize Yardian API communication."""
        super().__init__(
            hass,
            _LOGGER,
            config_entry=entry,
            name=entry.title,
            update_interval=SCAN_INTERVAL,
            always_update=False,
        )

        self.controller = controller
        self.yid = entry.data["yid"]
Repository: home-assistant/core
File: homeassistant/components/switchbot/config_flow.py (Python)

Code before:
            SupportedModels.LOCK,
            SupportedModels.LOCK_PRO,
            SupportedModels.LOCK_ULTRA,
        ):
            options.update(
                {
                    vol.Optional(
                        CONF_LOCK_NIGHTLATCH,
                        default=self.config_entry.options.get(
                            CONF_LOCK_NIGHTLATCH, DEFAULT_LOCK_NIGHTLATCH
                        ),
                    ): bool
                }
            )
        if (
            CONF_SENSOR_TYPE in self.config_entry.data
            and self.config_entry.data[CONF_SENSOR_TYPE] == SupportedModels.CURTAIN
        ):
            options.update(
                {
                    vol.Optional(
                        CONF_CURTAIN_SPEED,
                        default=self.config_entry.options.get(
                            CONF_CURTAIN_SPEED, DEFAULT_CURTAIN_SPEED
                        ),
                    ): vol.All(
                        vol.Coerce(int),
                        vol.Range(min=CURTAIN_SPEED_MIN, max=CURTAIN_SPEED_MAX),
                    )
                }
            )

        return self.async_show_form(step_id="init", data_schema=vol.Schema(options))


Review comment (refactor):
Consider using a number selector here so we can configure the interface to be a box or a slider depending on what's most appropriate.

https://github.com/home-assistant/core/blob/60014b6530fd1b5d6d0849de7ad4050a89eed329/homeassistant/helpers/selector.py#L1121

I'm not sure what the frontend does by default here.

Code after:
        ):
            options.update(
                {
                    vol.Optional(
                        CONF_LOCK_NIGHTLATCH,
                        default=self.config_entry.options.get(
                            CONF_LOCK_NIGHTLATCH, DEFAULT_LOCK_NIGHTLATCH
                        ),
                    ): bool
                }
            )
        if (
            CONF_SENSOR_TYPE in self.config_entry.data
            and self.config_entry.data[CONF_SENSOR_TYPE] == SupportedModels.CURTAIN
        ):
            options.update(
                {
                    vol.Optional(
                        CONF_CURTAIN_SPEED,
                        default=self.config_entry.options.get(
                            CONF_CURTAIN_SPEED, DEFAULT_CURTAIN_SPEED
                        ),
                    ): selector.NumberSelector(
                        selector.NumberSelectorConfig(
                            min=CURTAIN_SPEED_MIN,
                            max=CURTAIN_SPEED_MAX,
                            step=1,
                            mode=selector.NumberSelectorMode.SLIDER,
                        )
                    )
                }
            )

        return self.async_show_form(step_id="init", data_schema=vol.Schema(options))
Repository: home-assistant/core
File: homeassistant/components/flexit_bacnet/__init__.py (Python)

Code before:

    # Check if deprecated fireplace switch is enabled and create repair issue
    entity_reg = er.async_get(hass)
    fireplace_switch_id = f"{coordinator.device.serial_number}-fireplace_mode"

    # Look for the fireplace switch entity
    for entity in er.async_entries_for_config_entry(entity_reg, entry.entry_id):
        if entity.unique_id == fireplace_switch_id and not entity.disabled:
            # Switch is enabled, create deprecation issue
            climate_entity_id = entity.entity_id.replace("switch.", "climate.").replace(
                "_fireplace_mode", ""
            )
            async_create_issue(
                hass,
                DOMAIN,
                f"deprecated_switch_{entity.unique_id}",
                is_fixable=False,
                issue_domain=DOMAIN,
                severity=IssueSeverity.WARNING,
                translation_key="deprecated_fireplace_switch",
                translation_placeholders={
                    "entity_id": entity.entity_id,
                    "climate_entity_id": climate_entity_id,
                },
            )
            break

    return True


async def async_unload_entry(hass: HomeAssistant, entry: FlexitConfigEntry) -> bool:
    """Unload the Flexit Nordic (BACnet) config entry."""
    return await hass.config_entries.async_unload_platforms(entry, PLATFORMS)


Review comment (suggestion):
Okay so, people could have renamed the fireplace mode. Instead I would recommend to look up the entity_id via the entity registry and the unique id instead of just replacing the switch. Also since people can use translations and then fireplace_mode wouldn't be found

Code after:
"""The Flexit Nordic (BACnet) integration."""

from __future__ import annotations

from homeassistant.const import Platform
from homeassistant.core import HomeAssistant

from .coordinator import FlexitConfigEntry, FlexitCoordinator

PLATFORMS: list[Platform] = [
    Platform.BINARY_SENSOR,
    Platform.CLIMATE,
    Platform.NUMBER,
    Platform.SENSOR,
    Platform.SWITCH,
]


async def async_setup_entry(hass: HomeAssistant, entry: FlexitConfigEntry) -> bool:
    """Set up Flexit Nordic (BACnet) from a config entry."""

    coordinator = FlexitCoordinator(hass, entry)
    await coordinator.async_config_entry_first_refresh()

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

    return True


async def async_unload_entry(hass: HomeAssistant, entry: FlexitConfigEntry) -> bool:
    """Unload the Flexit Nordic (BACnet) config entry."""
    return await hass.config_entries.async_unload_platforms(entry, PLATFORMS)
Repository: home-assistant/core
File: homeassistant/components/traccar_server/__init__.py (Python)

Code before:

from datetime import timedelta

from aiohttp import CookieJar
from pytraccar import ApiClient

from homeassistant.config_entries import ConfigEntry
from homeassistant.const import (
    CONF_API_TOKEN,
    CONF_HOST,
    CONF_PASSWORD,
    CONF_PORT,
    CONF_SSL,
    CONF_USERNAME,
    CONF_VERIFY_SSL,
    Platform,
)
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import ConfigEntryAuthFailed
from homeassistant.helpers.aiohttp_client import async_create_clientsession
from homeassistant.helpers.event import async_track_time_interval

from .const import CONF_EVENTS, DOMAIN
from .coordinator import TraccarServerCoordinator

type TraccarServerConfigEntry = ConfigEntry[TraccarServerCoordinator]

PLATFORMS: list[Platform] = [
    Platform.BINARY_SENSOR,
    Platform.DEVICE_TRACKER,
    Platform.SENSOR,
]


async def async_setup_entry(
    hass: HomeAssistant, entry: TraccarServerConfigEntry
) -> bool:
    """Set up Traccar Server from a config entry."""
    if CONF_API_TOKEN not in entry.data:
        raise ConfigEntryAuthFailed(
            translation_domain=DOMAIN,
            translation_key="migrate_to_api_token",
        )
    client_session = async_create_clientsession(
        hass,
        cookie_jar=CookieJar(
            unsafe=not entry.data[CONF_SSL] or not entry.data[CONF_VERIFY_SSL]
        ),
    )
    coordinator = TraccarServerCoordinator(
        hass=hass,

Review comment (suggestion):
move it to the coordinator then you can avoid the `TYPE_CHECKING` everywhere

Code after:

from datetime import timedelta

from aiohttp import CookieJar
from pytraccar import ApiClient

from homeassistant.config_entries import ConfigEntry
from homeassistant.const import (
    CONF_API_TOKEN,
    CONF_HOST,
    CONF_PASSWORD,
    CONF_PORT,
    CONF_SSL,
    CONF_USERNAME,
    CONF_VERIFY_SSL,
    Platform,
)
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import ConfigEntryAuthFailed
from homeassistant.helpers.aiohttp_client import async_create_clientsession
from homeassistant.helpers.event import async_track_time_interval

from .const import CONF_EVENTS, DOMAIN
from .coordinator import TraccarServerConfigEntry, TraccarServerCoordinator

PLATFORMS: list[Platform] = [
    Platform.BINARY_SENSOR,
    Platform.DEVICE_TRACKER,
    Platform.SENSOR,
]


async def async_setup_entry(
    hass: HomeAssistant, entry: TraccarServerConfigEntry
) -> bool:
    """Set up Traccar Server from a config entry."""
    if CONF_API_TOKEN not in entry.data:
        raise ConfigEntryAuthFailed(
            translation_domain=DOMAIN,
            translation_key="migrate_to_api_token",
        )
    client_session = async_create_clientsession(
        hass,
        cookie_jar=CookieJar(
            unsafe=not entry.data[CONF_SSL] or not entry.data[CONF_VERIFY_SSL]
        ),
    )
    coordinator = TraccarServerCoordinator(
        hass=hass,
        config_entry=entry,
        client=ApiClient(
Repository: home-assistant/core
File: homeassistant/components/traccar_server/diagnostics.py (Python)

Code before:
                    "disabled": entity.disabled,
                    "unit_of_measurement": entity.unit_of_measurement,
                    "state": _entity_state(hass, entity, coordinator),
                }
                for entity in entities
            ],
        },
        KEYS_TO_REDACT,
    )


async def async_get_device_diagnostics(
    hass: HomeAssistant,
    entry: TraccarServerConfigEntry,
    device: dr.DeviceEntry,
) -> dict[str, Any]:
    """Return device diagnostics."""
    coordinator = entry.runtime_data
    entity_registry = er.async_get(hass)

    entities = er.async_entries_for_device(
        entity_registry,
        device_id=device.id,
        include_disabled_entities=True,
    )

    return async_redact_data(
        {
            "subscription_status": coordinator.client.subscription_status,
            "config_entry_options": dict(entry.options),
            "coordinator_data": coordinator.data,
            "entities": [
                {
                    "entity_id": entity.entity_id,
                    "disabled": entity.disabled,
                    "unit_of_measurement": entity.unit_of_measurement,
                    "state": _entity_state(hass, entity, coordinator),
                }
                for entity in entities
            ],
        },
        KEYS_TO_REDACT,
    )


Review comment (suggestion):
I don't know why that is here, but it changes the diagnostic snapshot, so it did something, please double check check

Code after:
                }
                for entity in entities
            ],
        },
        KEYS_TO_REDACT,
    )


async def async_get_device_diagnostics(
    hass: HomeAssistant,
    entry: TraccarServerConfigEntry,
    device: dr.DeviceEntry,
) -> dict[str, Any]:
    """Return device diagnostics."""
    coordinator = entry.runtime_data
    entity_registry = er.async_get(hass)

    entities = er.async_entries_for_device(
        entity_registry,
        device_id=device.id,
        include_disabled_entities=True,
    )

    await hass.config_entries.async_reload(entry.entry_id)

    return async_redact_data(
        {
            "subscription_status": coordinator.client.subscription_status,
            "config_entry_options": dict(entry.options),
            "coordinator_data": coordinator.data,
            "entities": [
                {
                    "entity_id": entity.entity_id,
                    "disabled": entity.disabled,
                    "unit_of_measurement": entity.unit_of_measurement,
                    "state": _entity_state(hass, entity, coordinator),
                }
                for entity in entities
            ],
        },
        KEYS_TO_REDACT,
    )
Repository: home-assistant/core
File: homeassistant/components/home_connect/common.py (Python)

Code before:
"""Common callbacks for all Home Connect platforms."""

from collections import defaultdict
from collections.abc import Callable
from functools import partial
from typing import cast

from aiohomeconnect.model import EventKey

from homeassistant.core import HomeAssistant
from homeassistant.helpers import entity_registry as er
from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback

from .coordinator import HomeConnectApplianceData, HomeConnectConfigEntry
from .entity import HomeConnectEntity, HomeConnectOptionEntity


def _create_option_entities(
    entity_registry: er.EntityRegistry,
    entry: HomeConnectConfigEntry,
    appliance: HomeConnectApplianceData,
    known_entity_unique_ids: dict[str, str],
    get_option_entities_for_appliance: Callable[
        [HomeConnectConfigEntry, HomeConnectApplianceData, er.EntityRegistry],
        list[HomeConnectOptionEntity],
    ],
    async_add_entities: AddConfigEntryEntitiesCallback,
) -> None:
    """Create the required option entities for the appliances."""
    option_entities_to_add = [
        entity
        for entity in get_option_entities_for_appliance(
            entry, appliance, entity_registry
        )
        if entity.unique_id not in known_entity_unique_ids
    ]
    known_entity_unique_ids.update(

Review comment (refactor):
To improve readability, I suggest adding the import `from homeassistant.helpers.entity_registry import EntityRegistry` as well and use it for typing purposes instead of `er.EntityRegistry`. It would be more consistent with the already existent code.

Code after:
"""Common callbacks for all Home Connect platforms."""

from collections import defaultdict
from collections.abc import Callable
from functools import partial
from typing import cast

from aiohomeconnect.model import EventKey

from homeassistant.const import Platform
from homeassistant.core import HomeAssistant
from homeassistant.helpers import entity_registry as er
from homeassistant.helpers.entity import EntityDescription
from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback

from .const import DOMAIN
from .coordinator import HomeConnectApplianceData, HomeConnectConfigEntry
from .entity import HomeConnectEntity, HomeConnectOptionEntity


def should_add_option_entity(
    description: EntityDescription,
    appliance: HomeConnectApplianceData,
    entity_registry: er.EntityRegistry,
    platform: Platform,
) -> bool:
    """Check if the option entity should be added for the appliance.

    This function returns `True` if the option is available in the appliance options
    or if the entity was added in previous loads of this integration.
    """
    description_key = description.key
    return description_key in appliance.options or (
        entity_registry.async_get_entity_id(
            platform, DOMAIN, f"{appliance.info.ha_id}-{description_key}"
        )
        is not None