New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use unicode slugify #19192

Merged
merged 20 commits into from Dec 17, 2018

Conversation

Projects
None yet
7 participants
@pvizeli
Copy link
Member

pvizeli commented Dec 11, 2018

Description:

Use mozilla slugify library with UTF-8 support:
https://github.com/mozilla/unicode-slugify

Fix: #19184

Update:
We need to move to https://github.com/un33k/python-slugify because the Mozilla PyPI packages are very outdated

@pvizeli pvizeli requested a review from home-assistant/core as a code owner Dec 11, 2018

@wafflebot wafflebot bot added the in progress label Dec 11, 2018

pvizeli added some commits Dec 11, 2018

@@ -14,6 +14,8 @@
from typing import (Any, Optional, TypeVar, Callable, KeysView, Union, # noqa
Iterable, List, Dict, Iterator, Coroutine, MutableSet)

from slugify import slugify

This comment has been minimized.

@balloob

balloob Dec 11, 2018

Member

I sense some naming issues ;)

This comment has been minimized.

@pvizeli

pvizeli Dec 11, 2018

Member

fixed :D

pvizeli added some commits Dec 11, 2018

@amelchio

This comment has been minimized.

Copy link
Member

amelchio commented Dec 11, 2018

I definitely have localized entity ids that could be improved.

Kinda hard to give comments though, some before/after examples would be nice ... maybe as tests?

@Anonym-tsk

This comment has been minimized.

Copy link
Contributor

Anonym-tsk commented Dec 11, 2018


return text
return moz_slug.slugify(
text, ok='_-', space_replacement='_', only_ascii=True)

This comment has been minimized.

@balloob

balloob Dec 11, 2018

Member

I don't think - is a valid entity id ?

@balloob

This comment has been minimized.

Copy link
Member

balloob commented Dec 11, 2018

Breaking change because any entity that does not have a unique ID and has foreign characters in their name will get a new entity ID.

pvizeli added some commits Dec 12, 2018

@pvizeli

This comment has been minimized.

Copy link
Member

pvizeli commented Dec 12, 2018

Maybe we need to move to https://github.com/un33k/python-slugify because the Mozilla PyPI packages are very outdated

pvizeli added some commits Dec 13, 2018

@pvizeli pvizeli changed the title RFC: Use mozilla slugify Use unicode slugify Dec 13, 2018

@balloob balloob referenced this pull request Dec 14, 2018

Closed

Fix creation entity_id for non-english names #19184

3 of 3 tasks complete
@balloob
Copy link
Member

balloob left a comment

Ok to merge when tests and typing fixed.

Show resolved Hide resolved tests/util/test_init.py Outdated
Show resolved Hide resolved tests/util/test_init.py Outdated
Show resolved Hide resolved tests/components/sensor/test_filesize.py Outdated
Show resolved Hide resolved tests/components/sensor/test_filesize.py Outdated
Show resolved Hide resolved tests/components/sensor/test_awair.py Outdated
Show resolved Hide resolved tests/components/sensor/test_awair.py Outdated
Show resolved Hide resolved tests/components/sensor/test_awair.py Outdated
Show resolved Hide resolved tests/components/sensor/test_awair.py Outdated
Show resolved Hide resolved tests/components/sensor/test_awair.py Outdated
Show resolved Hide resolved tests/components/device_tracker/test_meraki.py Outdated

pvizeli added some commits Dec 16, 2018

@pvizeli pvizeli merged commit 2bf36bb into dev Dec 17, 2018

3 of 5 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details
Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA

@delete-merged-branch delete-merged-branch bot deleted the mozilla-slugify branch Dec 17, 2018

@wafflebot wafflebot bot removed the in progress label Dec 17, 2018

dshokouhi added a commit to dshokouhi/home-assistant that referenced this pull request Dec 25, 2018

Use unicode slugify (home-assistant#19192)
* Update __init__.py

* Update setup.py

* Update requirements_all.txt

* Update __init__.py

* Update __init__.py

* Update __init__.py

* Update __init__.py

* Update __init__.py

* remove `-`

* fix packages

* Update package_constraints.txt

* Update __init__.py

* Update package_constraints.txt

* Update requirements_all.txt

* Update setup.py

* Fix tests

* Fix line issue

* fix all test

* fix type

* Fix lint
@syssi

This comment has been minimized.

Copy link
Member

syssi commented Jan 4, 2019

This change did break a lot of unique_ids in my setup:

# before
"unique_id": "coordination158d0002a4f9f8"

# after
"unique_id": "coordination_158d0002a4f9f8"

It looks like a "-" character was removed in the past. Now it's mapped from "-" to "_".

@balloob

This comment has been minimized.

Copy link
Member

balloob commented Jan 4, 2019

uhm, unique IDs should not get slugified. Maybe you want to add a PR that copies the old slugify function into the xiaomi_aqara component and use that to prevent a breaking change.

@syssi

This comment has been minimized.

Copy link
Member

syssi commented Jan 4, 2019

I will update the xiaomi_aqara component.

@syssi syssi referenced this pull request Jan 4, 2019

Merged

Don't slugify unique id #19770

@syssi

This comment has been minimized.

Copy link
Member

syssi commented Jan 6, 2019

The PR will break all entity ids (without unique_id) which are containing a minus sign "-". In the past this character was removed. In future it is mapped to an underscore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment