Fix: Assign consistent colors to all forecast models in utils.py #375
Conversation
Sukh-P
left a comment
Sukh-P
left a comment
There was a problem hiding this comment.
Just the tidy up changes needed then, I'll run the branch locally as well just to check it looks okay visually and then we can get this merged in, thanks for the work on this!
src/plots/utils.py
Outdated
| """Get colour from model label""" | ||
| if "PVLive" in model_name: | ||
| colour = colour_per_model.get(model_name, "#FFFFFF") | ||
| return colour_per_model.get(model_name, "#e4e4e4") |
There was a problem hiding this comment.
Can we keep the PVLive colour the same actually? It's white which is quite distinctive
src/plots/utils.py
Outdated
| } | ||
|
|
||
| # Make a cycle for extra models not in colour_per_model | ||
| # Skip first 3 colours as they are too similar to colours in colour_per_model |
There was a problem hiding this comment.
This comment should be removed too as the line it related to has been removed
|
@AnujNayak108 thanks again for your work on this, tried running this locally to check out the new colours and noticed some errors that the tests don't catch (which isn't great) Namely it's that the removed |
|
I’ve updated the plotting logic to use get_colour_from_model_name instead of line_color to handle color selection dynamically based on the model name. Please check if this works correctly in your environment. |
2 participants
Pull Request
Description
Fix consistent color assignment for all forecast models in plots/utils.py.
This ensures that each model has a unique, identifiable color in plots, including new or experimental models.
Fixes #373
How Has This Been Tested?
Ran test_colours.py to verify that all models, including new and PVLive models, are assigned a color.
Verified that colour_per_model dictionary updates correctly for new models.
If your changes affect data processing, have you plotted any changes? i.e. have you done a quick sanity check?
Checklist: