Skip to content
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

User messages and logging #73

Closed
dhblum opened this issue May 14, 2019 · 6 comments
Closed

User messages and logging #73

dhblum opened this issue May 14, 2019 · 6 comments

Comments

@dhblum
Copy link
Collaborator

dhblum commented May 14, 2019

BOPTEST needs a mechanism(s) to communicate warnings, errors, and other information with the user. Perhaps this could be done though logging, API messages, or other UI tools.

@dhblum
Copy link
Collaborator Author

dhblum commented May 24, 2022

@SenHuang19 I merged #272 and made some messaging and other clean up updates in 12cf85d. Can you make a PR to branch issue73_messaging with added unit tests?

I also think there are improvements (additional checks or check modifications) that could be made on the input checking and error reporting for each testcase.py function to be more effective and descriptive, see below. I do not think they are hard to implement now that you've got the general structure in place. I can also help implement these updates in parallel to you implementing unit tests on the existing. Then, we can add any needed additional unit tests to your initial set up. Let me know if you agree or other thoughts.

1. General

  • statuses of 400 or 500 should be logged as logger.error instead of logger.warning.

2. set_scenario

  • check "electricity_price" parameter is "constant", "dynamic", or "highly_dynamic"
  • check "time_period" parameter is an acceptable string based on those in days.json.
  • check if initialization actually throws error

3. set_forecast_parameters

  • check horizon and internal separately and return error message specific for each
  • check values are floats, rather than that they can be translated to floats

4. get_results

  • check start_time and final_time separately and return error message specific for each
  • check values are floats, rather than that they can be translated to floats
  • check var is valid point name and return error accordingly (check against inputs and measurements list)

5. get_measurements

  • use traceback in message like other if status is 500

6. get_inputs

  • use traceback in message like other if status is 500

7. set_step

  • check step is integer instead of can be translated to float
  • check step is non-negative

8. get_step

  • structure like get_inputs or other gets and similarly use traceback in message like other if status is 500

9. initialize

  • check start_time and warmup_period separately and return appropriate error message
  • check start_time and warmup_period are integer instead of can be translated to float
  • check start_time and warmup_period are non-negative

10. advance

  • check names are valid (use inputs list)
  • check key values are: if _activate should be bool or int, if _u should be float instead of can be translated to float
  • if min/max violated, send warning message that input value is truncated to min/max

@SenHuang19
Copy link
Contributor

@SenHuang19 I merged #272 and made some messaging and other clean up updates in 12cf85d. Can you make a PR to branch issue73_messaging with added unit tests?

I also think there are improvements (additional checks or check modifications) that could be made on the input checking and error reporting for each testcase.py function to be more effective and descriptive, see below. I do not think they are hard to implement now that you've got the general structure in place. I can also help implement these updates in parallel to you implementing unit tests on the existing. Then, we can add any needed additional unit tests to your initial set up. Let me know if you agree or other thoughts.

1. General

  • statuses of 400 or 500 should be logged as logger.error instead of logger.warning.

2. set_scenario

  • check "electricity_price" parameter is "constant", "dynamic", or "highly_dynamic"
  • check "time_period" parameter is an acceptable string based on those in days.json.
  • check if initialization actually throws error

3. set_forecast_parameters

  • check horizon and internal separately and return error message specific for each
  • check values are floats, rather than that they can be translated to floats

4. get_results

  • check start_time and final_time separately and return error message specific for each
  • check values are floats, rather than that they can be translated to floats
  • check var is valid point name and return error accordingly (check against inputs and measurements list)

5. get_measurements

  • use traceback in message like other if status is 500

6. get_inputs

  • use traceback in message like other if status is 500

7. set_step

  • check step is integer instead of can be translated to float
  • check step is non-negative

8. get_step

  • structure like get_inputs or other gets and similarly use traceback in message like other if status is 500

9. initialize

  • check start_time and warmup_period separately and return appropriate error message
  • check start_time and warmup_period are integer instead of can be translated to float
  • check start_time and warmup_period are non-negative

10. advance

  • check names are valid (use inputs list)
  • check key values are: if _activate should be bool or int, if _u should be float instead of can be translated to float
  • if min/max violated, send warning message that input value is truncated to min/max

Sure, will add unit tests and fix the issues above

@dhblum
Copy link
Collaborator Author

dhblum commented May 24, 2022

@SenHuang19 Ok thank you.

@SenHuang19
Copy link
Contributor

@dhblum Dave, could you please take a look at the PR #436? I believe we have addressed your comments. We also create unit test scripts for the 400 errors. Noted that we made minor changes to the unit test script so that each tests are independent with each other. Let me know if the change makes sense to you.

@dhblum
Copy link
Collaborator Author

dhblum commented Jun 21, 2022

@SenHuang19 Great, thanks so much. I will review as soon as I can and let you know.

@dhblum
Copy link
Collaborator Author

dhblum commented Jul 12, 2022

Closed by #441.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants