-
Notifications
You must be signed in to change notification settings - Fork 0
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
Extract unittests #19
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, it looks like an update to a few different things, could you comment on what you changed?
@@ -52,7 +52,7 @@ def fetch_data(self) -> Optional[Dict[str, Any]]: | |||
response.raise_for_status() | |||
return response.json() | |||
except requests.exceptions.RequestException as e: | |||
self.logger.error(f"An error occurred: {e}") | |||
self.logger.error("An error occurred: %s", e) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice updating all logs to use %s
self.logger.info("\n" + df.head().to_string()) | ||
self.logger.info("Time Period of Data:") | ||
self.logger.info(time_period) | ||
self.logger.debug("DataFrame of Demand Data:\n%s", df.to_string()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great reduction in Logging!
psycopg2 | ||
python-dotenv | ||
datetime | ||
anyio==4.4.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This brings up the question of getting the latest version of a product vs keeping the same version constantly. I don't know which answer is correct in this case. Ask the stakeholders?
|
||
from tests.mock_data.mock_dataframes import get_dated_mock_dataframe | ||
api_client_carbon.logger.error.assert_called_once_with("An error occurred: %s", mock_get.side_effect) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great catch of deleting the extra imports
Adds automated testing plus a badge.
Also adds a licence that can be linked-to by a badge.