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

Bool If statments #176

Closed GeoDerp closed 5 months ago

GeoDerp commented 5 months ago

Related to https://github.com/davidusb-geek/emhass/issues/154 , https://github.com/davidusb-geek/emhass/pull/174 , https://github.com/davidusb-geek/emhass/pull/175

Did a ruff Grep and found there is quite a flew boolean if statements, If The pr #175 does indeed to be the fix. A pull request could be done to replace all of these with if * == True: (Just to make sure this issue doesn't happen to anything else). If its not, or unnecessary/irrelevant in any way feel free to close the Issue.

> grep --include=\*.py -Rn -P '(if[\s][^\s]+:)' .
./scripts/load_clustering.py:51:    if data_path.is_file():
./scripts/load_forecast_sklearn.py:59:    if data_path.is_file():
./scripts/optim_results_analysis.py:47:    if get_data_from_file:
./scripts/optim_results_analysis.py:75:    if show_figures:
./scripts/optim_results_analysis.py:77:    if save_figures:
./scripts/optim_results_analysis.py:85:    if show_figures:
./scripts/optim_results_analysis.py:87:    if save_figures:
./scripts/optim_results_analysis.py:109:    # if show_figures:
./scripts/optim_results_analysis.py:111:    if save_figures:
./scripts/read_csv_plot_data.py:133:    if save_figs:
./scripts/read_csv_plot_data.py:147:    if save_figs:
./scripts/special_config_analysis.py:148:    if data_path.is_file():
./scripts/use_cases_analysis.py:75:    if save_figures:
./scripts/use_cases_analysis.py:85:    if save_figures:
./scripts/use_cases_analysis.py:94:    if save_figures:
./scripts/use_cases_analysis.py:105:    if save_figures:
./scripts/use_cases_analysis.py:121:    if save_figures:
./scripts/use_cases_analysis.py:141:    if save_figures:
./scripts/use_cases_analysis.py:162:    if save_figures:
./scripts/use_cases_analysis.py:170:    if save_figures:
./src/emhass/optimization.py:134:        if self.optim_conf['set_use_battery']:
./src/emhass/optimization.py:170:            if self.optim_conf['treat_def_as_semi_cont'][k]:
./src/emhass/optimization.py:190:        if self.optim_conf['set_use_battery']:
./src/emhass/optimization.py:210:            if self.optim_conf['set_total_pv_sell']:
./src/emhass/optimization.py:217:            if self.optim_conf['set_total_pv_sell']:
./src/emhass/optimization.py:233:        if self.optim_conf['set_use_battery']:
./src/emhass/optimization.py:309:            if self.optim_conf['treat_def_as_semi_cont'][k]:
./src/emhass/optimization.py:323:            if self.optim_conf['set_def_constant'][k]:
./src/emhass/optimization.py:351:        if self.optim_conf['set_use_battery']:
./src/emhass/optimization.py:353:            if self.optim_conf['set_nocharge_from_grid']:
./src/emhass/optimization.py:361:            if self.optim_conf['set_nodischarge_to_grid']:
./src/emhass/optimization.py:369:            if self.optim_conf['set_battery_dynamic']:
./src/emhass/optimization.py:453:        if self.optim_conf['set_use_battery']:
./src/emhass/optimization.py:472:        if self.optim_conf['set_total_pv_sell']:
./src/emhass/optimization.py:480:            if self.optim_conf['set_total_pv_sell']:
./src/emhass/optimization.py:487:            if self.optim_conf['set_total_pv_sell']:
./src/emhass/optimization.py:504:        if debug:
./src/emhass/optimization.py:639:        Helper function to validate (and if necessary: correct) the defined optimization window of a deferrable load.
./src/emhass/retrieve_hass.py:105:                    if minimal_response: # A support for minimal response
./src/emhass/retrieve_hass.py:107:                    if significant_changes_only: # And for signicant changes only (check the HASS restful API for more info)
./src/emhass/retrieve_hass.py:181:            if load_negative: # Apply the correct sign to load power
./src/emhass/retrieve_hass.py:188:        if set_zero_min: # Apply minimum values
./src/emhass/retrieve_hass.py:327:        if self.get_data_from_file:
./src/emhass/retrieve_hass.py:334:        if response.ok:
./src/emhass/forecast.py:453:        if set_mix_forecast:
./src/emhass/forecast.py:584:            if self.get_data_from_file:
./src/emhass/forecast.py:608:                if filename_path.is_file():
./src/emhass/forecast.py:614:            if use_last_window:
./src/emhass/forecast.py:658:        if set_mix_forecast:
./src/emhass/web_server.py:126:    if args.addon==1:
./src/emhass/web_server.py:133:        if options_json.exists():
./src/emhass/web_server.py:140:        if use_options:
./src/emhass/web_server.py:144:            if options_json.exists():
./src/emhass/web_server.py:158:    if config_path.exists():
./src/emhass/web_server.py:181:    if args.addon==1:
./src/emhass/web_server.py:219:    if use_options:
./src/emhass/utils.py:54:    if save_to_file:
./src/emhass/utils.py:407:    if use_secrets:
./src/emhass/utils.py:419:    if use_secrets:
./src/emhass/command_line.py:72:        if get_data_from_file:
./src/emhass/command_line.py:104:        if get_data_from_file:
./src/emhass/command_line.py:136:        if get_data_from_file:
./src/emhass/command_line.py:201:    if save_data_to_file:
./src/emhass/command_line.py:237:    if save_data_to_file:
./src/emhass/command_line.py:281:    if save_data_to_file:
./src/emhass/command_line.py:352:        if filename_path.is_file():
./src/emhass/command_line.py:359:    if use_last_window:
./src/emhass/command_line.py:370:    if model_predict_publish:
./src/emhass/command_line.py:413:        if filename_path.is_file():
./src/emhass/command_line.py:446:    if save_data_to_file:
./src/emhass/command_line.py:503:    if input_data_dict['opt'].optim_conf['set_use_battery']:
./src/emhass/command_line.py:631:        if args.debug:
./src/emhass/command_line.py:638:        if args.debug:
./tests/test_forecast.py:50:        if self.get_data_from_file:
./tests/test_forecast.py:207:        if self.get_data_from_file:
./tests/test_forecast.py:282:        if self.get_data_from_file:
./tests/test_optimization.py:32:        if get_data_from_file:
./tests/test_retrieve_hass.py:31:        if get_data_from_file:
./tests/test_retrieve_hass.py:39:            if save_data_to_file:
davidusb-geek commented 5 months ago

Yes of course like I said this is basic Python so I use it extensively. But maybe there are some hidden dragons when retrieving data from flask end points, etc...

GeoDerp commented 5 months ago

It may be good to close this issue and keep an eye open to see if there is any new issue that arrives that seems similar and go from there. Otherwise if you like we can start transitioning the if statements (ex. Anything in the /scripts that may use passed parameters)

davidusb-geek commented 5 months ago

The latest PR #175 did not solve the issue. I've just tested it using a local compile of the standalone version and it is not taking into account the perform_backtest parameter. It seems to be a problem with bool only. Because I tested this and it changed the model type just fine:

curl -i -H "Content-Type:application/json" -X POST -d '{"sklearn_model": "LinearRegression","perform_backtest": "true"}' http://localhost:5000/action/forecast-model-fit
GeoDerp commented 5 months ago

Sorry about that. I'll quickly run some more tests now if I see if I can't diagnose the issue before I head to bed

GeoDerp commented 5 months ago

So the veritable is acting as a string (atleast in machine_learning_forecaster.py). so if perform_backtest == "True" or perform_backtest == "true": will work Going to spend a sec to work out why

davidusb-geek commented 5 months ago

Maybe try to convert to bool when retrieving runtime params in utils.py?

GeoDerp commented 5 months ago

Maybe try to convert to bool when retrieving runtime params in utils.py?

Exactly what I was thing about @davidusb-geek !

my idea was:

  if 'perform_backtest' not in runtimeparams.keys():
            perform_backtest = False
        else:
            perform_backtest = eval(str(runtimeparams['perform_backtest']).capitalize())
        params['passed_data']['perform_backtest'] = perform_backtest
GeoDerp commented 5 months ago

This might have to be done on all bool runtimes

GeoDerp commented 5 months ago

177

GeoDerp commented 5 months ago

Sorry about that @davidusb-geek , that was on me again for not testing well enough. The pull request is ruff but hopefully you can run with it and fix the bug. I'm heading to bed 👍

davidusb-geek commented 5 months ago

Sorry about that @davidusb-geek , that was on me again for not testing well enough. The pull request is ruff but hopefully you can run with it and fix the bug. I'm heading to bed 👍

Just tested this and works fine now! Releasing a patch soon...