Get area and floor by alias #126150

formatBCE · 2024-09-17T20:02:31Z

Proposed change

Add ability to get area and floor by alias, when using corresponding get_$_by_name function.
This is another take on changes, proposed in this PR by @synesthesiam

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue:
Link to documentation pull request: Add alias as accepted lookup value for area_id and floor_id templates home-assistant.io#34846

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
I have followed the perfect PR recommendations
The code has been formatted using Ruff (ruff format homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

formatBCE · 2024-09-17T20:38:47Z

@balloob you commented on previous PR by Mike, could you check this one please?

arturpragacz

Some first look issue. Same for floors.

arturpragacz · 2024-09-20T16:17:41Z

homeassistant/helpers/area_registry.py

    @callback
    def async_get_area_by_name(self, name: str) -> AreaEntry | None:
-        """Get area by name."""
-        return self.areas.get_by_name(name)
+        """Get area by name or alias."""
+        if area := self.areas.get_by_name(name):
+            return area
+        areas_list = self.areas.get_areas_for_alias(name)
+        return areas_list[0] if areas_list else None


This method is used by other methods in this class, like async_create, therefore you cannot change it here without consequences.

What you probably wanted to do is to create a separate method for your new purpose and call it in templates.

This method is used by other methods in this class, like async_create, therefore you cannot change it here without consequences.

What you probably wanted to do is to create a separate method for your new purpose and call it in templates.

That's what i suggested in Discord to devs, but we agreed to use same method.

P.S. here's Discord link

Who is devs? Who is we?

If there are some additional decisions made, they should be documented in the PR.

The code, as it is now, is buggy, and cannot be merged in this state.

Who is devs? Who is we?

If there are some additional decisions made, they should be documented in the PR.

The code, as it is now, is buggy, and cannot be merged in this state.

I posted link to Discord discussion.

As you may see in description, this approach was originally proposed by @synesthesiam . However, that PR was closed because it was using unoptimized for-loops. I used index array for it, but the logic remained the same.

I see now that in the previous PR balloob had the same reservations about this approach as I do. This cannot be merged like that.

If you want to, we can discuss it further on Discord.

I see now that in the previous PR balloob had the same reservations about this approach as I do. This cannot be merged like that.

If you want to, we can discuss it further on Discord.

I'd love to! :) I actually missed that comment by Paulus!

I wrote to you on Discord.

arturpragacz

LGTM

formatBCE · 2025-01-25T23:39:06Z

@frenck @balloob sorry for ping - this was marked for 2024.12, but looks like didn't make it. Should i close PRs, or they'll be merged eventually?

joostlek · 2025-01-25T23:42:25Z

Please be patient, I'm still in the process of trying to get the right people to look at it

arturpragacz · 2025-01-26T00:01:13Z

Oh, it has been quite some time. And some changes along the way.

Still I think it looks OK.

But please make sure it merges cleanly on current dev and CI still passes.

formatBCE · 2025-01-26T00:03:25Z

Oh, it has been quite some time. And some changes along the way.

Still I think it looks OK.

But please make sure it merges cleanly on current dev and CI still passes.

Want me to merge dev?

arturpragacz · 2025-01-26T00:05:38Z

Yes. It's been 4 months, there's been some changes, let's make sure CI still passes.

formatBCE · 2025-01-26T00:25:14Z

Yes. It's been 4 months, there's been some changes, let's make sure CI still passes.

Remember, that CI can fail randomly too :)

formatBCE · 2025-01-26T04:31:43Z

@arturpragacz your changes to the name normalization removed normalize_name method, that i used for aliases too. Should i use normalize_name from normalized_name_base_registry.py or create own (duplicated) implementation as in timers.py and intent.py?

arturpragacz · 2025-01-26T20:00:35Z

Use the one from the registry.

formatBCE · 2025-01-27T05:28:54Z

Done.

formatBCE · 2025-02-25T20:39:38Z

Please be patient, I'm still in the process of trying to get the right people to look at it

Any news? :) Another month...

frenck · 2025-03-02T14:28:01Z

Hi there @formatBCE 👋

Thanks for the pull request. I've marked it as draft for a moment. I want to take this one into the core architectural meeting to check with the others. We had a discussion on the linked previous pull request by Mike, so I want to make sure all concerns are addressed in this pull request.

Will get back to you later this week 👍

../Frenck

frenck

Thanks for your patience on this one @formatBCE 🙏

We have discussed it with the team, and agreed this approach is acceptable. Reviewed the implementation and looks good as well 👍

Thanks for this one 🥇

../Frenck

formatBCE · 2025-03-27T12:39:17Z

Woohoo! Glad to help! :)

formatBCE requested a review from a team as a code owner September 17, 2024 20:02

home-assistant bot added cla-signed core new-feature labels Sep 17, 2024

formatBCE marked this pull request as draft September 17, 2024 21:10

formatBCE marked this pull request as ready for review September 17, 2024 21:10

synesthesiam self-assigned this Sep 18, 2024

arturpragacz suggested changes Sep 20, 2024

View reviewed changes

formatBCE mentioned this pull request Sep 20, 2024

Add alias as accepted lookup value for area_id and floor_id templates home-assistant/home-assistant.io#34846

Merged

8 tasks

formatBCE requested a review from arturpragacz September 20, 2024 22:15

arturpragacz approved these changes Sep 21, 2024

View reviewed changes

frenck requested a review from balloob November 10, 2024 20:21

frenck modified the milestone: 2024.12.0b0 Nov 25, 2024

formatBCE requested a review from arturpragacz February 5, 2025 21:41

arturpragacz approved these changes Feb 6, 2025

View reviewed changes

frenck marked this pull request as draft March 2, 2025 14:26

frenck marked this pull request as ready for review March 27, 2025 11:50

formatBCE added 5 commits March 27, 2025 12:50

Add possibility to get area by alias

e7c63d2

Add ability to get floor by alias

766ae4e

Moved alias lookup to separate function, adjusted templates.

26e18e7

Changed registry to return all areas/floors with given alias

e2f92c0

Use normalize_name from normalized_name_base_registry

ec0ca01

frenck force-pushed the get_area_and_floor_by_alias branch from 2685503 to ec0ca01 Compare March 27, 2025 11:51

frenck approved these changes Mar 27, 2025

View reviewed changes

frenck merged commit dea00fa into home-assistant:dev Mar 27, 2025
48 checks passed

github-actions bot locked and limited conversation to collaborators Mar 28, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get area and floor by alias #126150

Get area and floor by alias #126150

formatBCE commented Sep 17, 2024 •

edited

Loading

formatBCE commented Sep 17, 2024

arturpragacz left a comment

arturpragacz Sep 20, 2024

formatBCE Sep 20, 2024 •

edited

Loading

arturpragacz Sep 20, 2024

formatBCE Sep 20, 2024

arturpragacz Sep 20, 2024

formatBCE Sep 20, 2024

arturpragacz Sep 20, 2024

arturpragacz left a comment

formatBCE commented Jan 25, 2025

joostlek commented Jan 25, 2025

arturpragacz commented Jan 26, 2025

formatBCE commented Jan 26, 2025

arturpragacz commented Jan 26, 2025

formatBCE commented Jan 26, 2025

formatBCE commented Jan 26, 2025

arturpragacz commented Jan 26, 2025

formatBCE commented Jan 27, 2025

formatBCE commented Feb 25, 2025

frenck commented Mar 2, 2025

frenck left a comment

formatBCE commented Mar 27, 2025

Get area and floor by alias #126150

Get area and floor by alias #126150

Conversation

formatBCE commented Sep 17, 2024 • edited Loading

Proposed change

Type of change

Additional information

Checklist

formatBCE commented Sep 17, 2024

arturpragacz left a comment

Choose a reason for hiding this comment

arturpragacz Sep 20, 2024

Choose a reason for hiding this comment

formatBCE Sep 20, 2024 • edited Loading

Choose a reason for hiding this comment

arturpragacz Sep 20, 2024

Choose a reason for hiding this comment

formatBCE Sep 20, 2024

Choose a reason for hiding this comment

arturpragacz Sep 20, 2024

Choose a reason for hiding this comment

formatBCE Sep 20, 2024

Choose a reason for hiding this comment

arturpragacz Sep 20, 2024

Choose a reason for hiding this comment

arturpragacz left a comment

Choose a reason for hiding this comment

formatBCE commented Jan 25, 2025

joostlek commented Jan 25, 2025

arturpragacz commented Jan 26, 2025

formatBCE commented Jan 26, 2025

arturpragacz commented Jan 26, 2025

formatBCE commented Jan 26, 2025

formatBCE commented Jan 26, 2025

arturpragacz commented Jan 26, 2025

formatBCE commented Jan 27, 2025

formatBCE commented Feb 25, 2025

frenck commented Mar 2, 2025

frenck left a comment

Choose a reason for hiding this comment

formatBCE commented Mar 27, 2025

formatBCE commented Sep 17, 2024 •

edited

Loading

formatBCE Sep 20, 2024 •

edited

Loading