Conversation
- Add heatmap_color_scale in graph_utils (light-to-dark blue) - Replace px.colors.sequential.deep with heatmap_color_scale in all three heatmaps (contribution, contributor, reviewer file heatmaps) - Aligns heatmap colors with app theme and other visualizations Made-with: Cursor
- Replace raw layout dict updates with fig.update_layout() - Add font=dict(size=14), axis titles to match other visualizations - Style coloraxis colorbar (tick/title font white) for dark theme Made-with: Cursor
- Add create_heatmap_figure() in graph_utils for common imshow + layout - Refactor all three heatmaps to use helper; remove duplicated layout code - Drop unused plotly.express import from heatmap modules Made-with: Cursor
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| from dateutil.relativedelta import * # type: ignore | ||
| import plotly.express as px | ||
| from pages.utils.graph_utils import get_graph_time_values, color_seq | ||
| from pages.utils.graph_utils import create_heatmap_figure, get_graph_time_values, color_seq |
There was a problem hiding this comment.
[pylint] reported by reviewdog 🐶
W0611: Unused get_graph_time_values imported from pages.utils.graph_utils (unused-import)
| from dateutil.relativedelta import * # type: ignore | ||
| import plotly.express as px | ||
| from pages.utils.graph_utils import get_graph_time_values, color_seq | ||
| from pages.utils.graph_utils import create_heatmap_figure, get_graph_time_values, color_seq |
There was a problem hiding this comment.
[pylint] reported by reviewdog 🐶
W0611: Unused color_seq imported from pages.utils.graph_utils (unused-import)
| from dateutil.relativedelta import * # type: ignore | ||
| import plotly.express as px | ||
| from pages.utils.graph_utils import get_graph_time_values, color_seq | ||
| from pages.utils.graph_utils import create_heatmap_figure, get_graph_time_values, color_seq |
There was a problem hiding this comment.
[pylint] reported by reviewdog 🐶
W0611: Unused get_graph_time_values imported from pages.utils.graph_utils (unused-import)
| from dateutil.relativedelta import * # type: ignore | ||
| import plotly.express as px | ||
| from pages.utils.graph_utils import get_graph_time_values, color_seq | ||
| from pages.utils.graph_utils import create_heatmap_figure, get_graph_time_values, color_seq |
There was a problem hiding this comment.
[pylint] reported by reviewdog 🐶
W0611: Unused color_seq imported from pages.utils.graph_utils (unused-import)
EngCaioFonseca
moved this from Backlog
to Testing | Waiting on PR response | Blocked
in Aspen Project Board
|
updated title so its clear that this isnt a conflict with #1087 |
|
it seems like this PR doesnt have a lot of details in the PR description, but the commits make it fairly clear that its meant to be a styling PR for the figures and graphs for the heatmap. Im generally okay with the concept, although I'd be curious to learn more about why the decision was made to refactor some of the figure creation into a shared function between these two graphs. Is this done in other places in the code? or do we want to keep each visualization largely independent from a code-imports standpoint? |
MoralCode
left a comment
MoralCode
left a comment
There was a problem hiding this comment.
left general comments above
|
@EngCaioFonseca Does this pr need to be merged before #1106 ? |
|
Closed due to #1106 |
github-project-automation
bot
moved this from Testing | Waiting on PR response | Blocked
to Done
in Aspen Project Board
3 participants
Generative AI disclosure
Please select one option:
If AI tools were used, please provide details below:
- What tools were used? Claude
- How were these tools used? Initial Draft
- Did you review these outputs before submitting this PR? Yes, reviewed, tested.