Hello 🙂 I would like to add a PR to

[gusty badger]

You could also do

@callback
def async_abort_entries_match(
    self, match_dict: dict[str, 
Any] | None = None
) -> None:
    ...

[coral fossil]

You mean like adding

@callback
def async_abort_entries_match(
    other_entries: list[ConfigEntry], match_dict: dict[str, Any] | None = None
) -> None:
    _async_abort_entries_match(other_entries, match_dict)

to the config_entries.py?

[gusty badger]

@callback
def async_abort_entries_match(
    self, match_dict: dict[str, Any] | None = None
) -> None:
    ...

@callback
def _async_abort_entries_match(
    self, match_dict: dict[str, Any] | None = None
) -> None:

eg https://github.com/home-assistant/core/blob/b34302e51bf0a6fac5ac00d0dbaa8d87a317f006/homeassistant/core.py#L559

[coral fossil]

Ah, but I think this syntax only works when having functions with the same name, but different parameters and/or return type

There are two possible ways to achieve this:

1. Keep the old name but add the new name as a assignment (or rename the function and add the old name as an assignment)

    @callback
    def _async_abort_entries_match(
        self, match_dict: dict[str, Any] | None = None
    ) -> None:
        # original function body

    async_abort_entries_match = _async_abort_entries_match

2. Add a function which calls the other function

    @callback
    def _async_abort_entries_match(
        self, match_dict: dict[str, Any] | None = None
    ) -> None:
        # original function body

    @callback
    def async_abort_entries_match(
        self, match_dict: dict[str, Any] | None = None
    ) -> None:
        self._async_abort_entries_match(match_dict)

[gusty badger]

I tend to say, choose your prefred one and create a PR for it 🙂

[errant rover]

Why not just disable the complaint and keep it as is?

[coral fossil]

That's of course possible as well, my idea was to have it "cleaner" by not always having to add # pylint: disable-next=protected-access where it is used

[void oasis]

Why would we even add this in the first place?

as in, you write: "But I would like to discuss it a bit before opening a PR."

but not context, motivation or reasoning to even consider anything like this has been given

[coral fossil]

For example here https://github.com/home-assistant/core/blob/41215aa95410c25d84b6cfec4a0da93be3ca45b1/homeassistant/components/webmin/config_flow.py#L37-L40, when using a schema config flow the functions need to be accessed via handler.parent_handler._async_abort_entries_match() and handler.parent_handler._abort_if_unique_id_configured(), which is of course possible, but pylint doesn't like it because it is technically accessing a protected function.

[void oasis]

Well we are moving away from that, as we have introduce a reconfigure step

So, there is currently a discussion to forbid the use of update on abort in user flows

I see this is just for aborting it

but seems odd to do that in user input validation tbh

as that is no longer about validating user input

Looking at the linked config flow

I wonder why it is using the SchemaConfigFlow at all?

[lapis radish]

One thing that I found interesting in the webmin config flow is that everything is stored in the options and not in the data dict

[void oasis]

Oh good point

that needs to be fixed

[coral fossil]

Is options the wrong dict?

[void oasis]

It is a result of using this schema helper... this helper is meant to be used by helpers

not integrations

for helpers, the options are used, to provide the UI from the entity config panel (they basically have no connection/auth information)

[coral fossil]

Ah, good to know

[void oasis]

Yes, connection and authentication information belongs in the data dict

[coral fossil]

Okay, then I will fix it

[coral fossil]

Ah, the SchemaConfigFlow fills the options dict with the user input and sets the data dict to {}, that's why I used the options dict

[coral fossil]

I chose it because it is a simple and easy way for a config flow. When not much logic is needed, it is very intuitive to use and implement.

[void oasis]

Right, but not meant for the case you are using

The schema flow was created for managing things like groups

[coral fossil]

Mhm, I see