search

davidusb-geek / emhass

emhass: Energy Management for Home Assistant, is a Python module designed to optimize your home energy interfacing with Home Assistant.
MIT License
260 stars 51 forks source link

Fixed bug in #169 #181

Open GeoDerp opened 5 months ago

GeoDerp commented 5 months ago

Related to, and possible fix for issue: https://github.com/davidusb-geek/emhass-add-on/issues/74

codecov[bot] commented 5 months ago

Codecov Report

Attention: Patch coverage is 98.61111% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 87.75%. Comparing base (c9686cd) to head (910c26d). Report is 158 commits behind head on master.

:exclamation: Current head 910c26d differs from pull request most recent head 682a1d7. Consider uploading reports for the commit 682a1d7 to get more accurate results

Files Patch % Lines
src/emhass/utils.py 98.61% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #181 +/- ## =========================================== + Coverage 73.21% 87.75% +14.53% =========================================== Files 7 6 -1 Lines 2035 1707 -328 =========================================== + Hits 1490 1498 +8 + Misses 545 209 -336 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

GeoDerp commented 5 months ago

Ill see if I can resolve these test errors. Its a weird one. Sometimes its fine, sometimes not?
Edit: @davidusb-geek, could you run the test again?

davidusb-geek commented 5 months ago

This PR reinstates the associations list and dict and solves the issue that arised when we released this method. Is that right?

GeoDerp commented 5 months ago

In theory yes 👍

GeoDerp commented 4 months ago

This PR will need to be tested to see if its still functional and dosn't generate any bugs.

davidusb-geek commented 4 months ago

This PR will need to be tested to see if its still functional and dosn't generate any bugs.

Ok, we will probably integrate this in a future 0.9.0 release along with the new MLRegression class. But yes will need to be tested.

Also requirements_addon.txt file is no longer needed right?

GeoDerp commented 4 months ago

This PR will need to be tested to see if its still functional and dosn't generate any bugs.

Ok, we will probably integrate this in a future 0.9.0 release along with the new MLRegression class. But yes will need to be tested.

Also requirements_addon.txt file is no longer needed right?

Oh sorry. Yeah I don't believe so. Does the web server requirements only being used for the devcontainer? If so, could that be removed also. (replaced with manual run pip installs?)

GeoDerp commented 4 months ago

It may be good to keep the web server requirements if you want to specify the version of flask, waitress and ploty. I'll change the Docker to use the file if that's the case? @davidusb-geek

davidusb-geek commented 4 months ago

It may be good to keep the web server requirements if you want to specify the version of flask, waitress and ploty. I'll change the Docker to use the file if that's the case? @davidusb-geek

Yes we will keep the web server requirements and erase the other requirements_addon file

GeoDerp commented 4 months ago

It may be good to keep the web server requirements if you want to specify the version of flask, waitress and ploty. I'll change the Docker to use the file if that's the case? @davidusb-geek

Yes we will keep the web server requirements and erase the other requirements_addon file

Making a PR now.