-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
Changes minutes to hours on forecast horizons tables/plots #177
base: main
Are you sure you want to change the base?
Conversation
Changed minutes to hours on forecast horizons tables/plots.
Thanks for this, would you also be able to change it here |
Apologies for the delayed response. I've made the suggested changes and would appreciate your review. |
@@ -133,14 +133,28 @@ def metric_page(): | |||
make_recent_summary_stats(values=y_mae) | |||
make_recent_summary_stats(values=y_rmse, title="Recent RMSE") | |||
|
|||
def convert_minutes_to_hours(minutes): | |||
return round(minutes / 60, 1) |
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.
we should probably only define this once
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.
perhaps import from src/plots/forecast_horizon
Could you add a screen shot to the PR? Or perhaps you didnt run this locally? |
I didn't run this locally. I was trying to setup it locally but I guess only OCF team members can connect to the forecast development database using these Notion instructions(as mentioned in readme). |
@peterdudfield Could I kindly request access to the database to resolve this issue? If that's not feasible, would it be better to close the pull request, or is there an alternative approach that I could take? |
Sorry, thats not feasible, you could run your own database using nowcasting_Datamodel, but the database will be empty |
Pull Request
Description
Changed minutes to hours on forecast horizons tables/plots.
Fixes #167
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.
Please also list any relevant details for your test configuration
If your changes affect data processing, have you plotted any changes? i.e. have you done a quick sanity check?
Checklist: