Allow translations for enum values in EnumSensor and ZHAEnumSelectEntity entities #86
Conversation
…rting with a capital letter'
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #86 +/- ##
=======================================
Coverage 95.64% 95.64%
=======================================
Files 61 61
Lines 9232 9237 +5
=======================================
+ Hits 8830 8835 +5
Misses 402 402 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
| self._translation_keys = { | ||
| entry.name.lower(): entry.name for entry in self._enum | ||
| } | ||
| self._attr_options = [entry.name.lower() for entry in self._enum] |
There was a problem hiding this comment.
Will this be a breaking change for entities that do not yet implement translation keys?
There was a problem hiding this comment.
In homeassistant/core, I have written translation keys for all enums. But unfortunately, regardless of this specific code fragment, the internal state will change from the current form (titles with the first letter being a capital) to snake_case without any capitals. Automations that depend on this will seize to function.
There was a problem hiding this comment.
Hmm. If it's going to be a breaking change we should get these strings into the exact state we want them (with no compatibility shims) and then prominently mark the breaking change as such.
There was a problem hiding this comment.
Currently, the only thing we do is .lower() as most Enums have capitals in them. Do you see that as a compatibility shim? Changing that requires changes in at least: zigpy/zha, zigpy/zigpy and zigpy/zha-device-handlers. There might be more users of zigpy/zigpy that rely on the names in the Enums.
There was a problem hiding this comment.
Secondly, if we want to create final names, we probably should change the CamelCase Enums to snake_case (CamelCase looks pretty bad in lowercase), which aren't many, but they do exist in zigpy/zha and zigpy/zha-device-handlers.
Thirdly, maybe this is also a nice opportunity to move the Enums still present in zigpy/zha to zigpy/zha-device-handlers, while changing the naming where appropriate.
There was a problem hiding this comment.
SomeValue would become somevalue instead of some_value. I think in zigpy we use a mix of UPPERCASE_WITH_UNDERSCORES, CamelCase, and Whatever_case_this_is.
There was a problem hiding this comment.
Ok, I see. They're all still valid at least 😄
So, for HA state/translation keys, we probably always want some_value.
Which style do we want to use for the actual enums in general? Also some_value?
Or do we want to leave them as-is for now and just convert all types you mentioned to some_value in ZHA? Should be possible at least.. (We could change the actual enums to some_value (or similar) later, without breaking HA state again then.)
There was a problem hiding this comment.
When leaving the actual enums as-is, from "SomeValue" we would get "somevalue" using .lower(). So when changing the actual enums later on to "some_value", we would still have a breaking change. Although we could also do something like: re.sub(r"_+", r"_", re.sub(r"([A-Z])", r"_\1", "SomeValue").strip("_").lower()).
Edit: fix multiple underscores for occurrences of "Some_Value".
There was a problem hiding this comment.
Here's all of the enum members defined in quirks and zigpy: https://gist.github.com/puddly/2190cc8450bbcb6200afd6b87e516252.
I think the function in there should work, I didn't see any mistakes for this mapping:
{
"FooBar": "foo_bar",
"FOOBAR": "foobar",
"FOO_BAR": "foo_bar",
"foo_bar": "foo_bar",
"FOOBar": "foo_bar"
}There was a problem hiding this comment.
Yeah, let's use this.
This is the counterpart of the original in homeassistant/core: home-assistant/core#109309