Best effort state initialization of bayesian binary sensor#30962
Conversation
jlmcgehee21
force-pushed
the
fix/bayesian_binary_initialization
branch
3 times, most recently
from
50a43d3 to
43bf9a7
Compare
MartinHjelmare
changed the title
jlmcgehee21
force-pushed
the
fix/bayesian_binary_initialization
branch
from
d1d5f49 to
b87f54c
Compare
bachya
left a comment
bachya
left a comment
There was a problem hiding this comment.
Code looks good to me. I ran this in a local HASS and it appears to work as expected. I approve pending CI passing.
|
Sorry for the communication gap here -- do I actually need to increase test coverage to get this through? My change ended up adding a new test case to test the functionality. The decrease in coverage is an artifact of a minor refactor. I don't think I added anything new that's untested. Definitely willing to add more coverage is that's a true blocker, not trying to push against it. I've just got a lot of things on my todo list, and interested in checking this off. |
|
@jlmcgehee21 You don't need to add new test coverage, but this PR drops it by 2%, so it's not maintaining existing coverage. |
|
So if I don’t touch this, it will get merged in the next release? |
|
No. A PR can't drop coverage below 94%. |
|
Ok thank you for the clarification. I’ll fix. |
jlmcgehee21
force-pushed
the
fix/bayesian_binary_initialization
branch
from
8edc139 to
8d7d168
Compare
|
@jlmcgehee21 please resolve the merge conflict :) |
Why: * home-assistant#30119 This change addresses the need by: * Running the main update logic eagerly for each entity being observed on `async_added_to_hass`. * Test of the new behavior.
mutate instance attributes.
Why: * Because for some reason my commits decreased test coverage. This change addresses the need by: * Adding coverage for the case where a device returns `STATE_UNKNOWN` * Adding coverage for configurations with templates
jlmcgehee21
force-pushed
the
fix/bayesian_binary_initialization
branch
from
8d7d168 to
dbfbc8a
Compare
|
I just deleted some comments and got failed tests in some unrelated code concerning timezones. Didn't investigate in depth, but it may be due to daylight savings last night. |
|
Can someone with authority, re-run these tests? I believe the failure was related to daylight savings (see previous comment) @bachya @springstan |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Thank you! 🎉 |
|
👍 -- sorry for the long turnaround. Glad I was able to chip away and give back to a project that has given me so much! |
| print(self.current_observations) | ||
| print(self.observations_by_entity) |
7 participants
Why:
This change addresses the need by:
on
async_added_to_hass.Breaking Change:
Description:
Related issue (if applicable): fixes #
Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#<home-assistant.io PR number goes here>
Example entry for
configuration.yaml(if applicable):Checklist:
tox. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
python3 -m script.hassfest.requirements_all.txtby runningpython3 -m script.gen_requirements_all..coveragerc.If the code does not interact with devices: