Skip to content

fix(legend): position chart legends outside plot area to prevent overlap#89

Merged
exploreriii merged 2 commits intohiero-hackers:mainfrom
Adityarya11:feat/fix-legend-bug#70
Apr 1, 2026
Merged

fix(legend): position chart legends outside plot area to prevent overlap#89
exploreriii merged 2 commits intohiero-hackers:mainfrom
Adityarya11:feat/fix-legend-bug#70

Conversation

@Adityarya11
Copy link
Copy Markdown
Contributor

Changes

  • Bar and Line Charts:
    • Legend anchor changed to the right outside the plot (legend_bbox_to_anchor=(1.02, 1), legend_loc="upper left").
    • layout_rect adjusted to reserve margin on the right (layout_rect=(0, 0, 0.85, 1)), guaranteeing that the legend has a fixed area and won’t obscure chart content.
  • General:
    • No visual overlap between legends and axis labels, even with rotated tick labels or a large number of series.
    • The update is minimal and follows the project’s existing layout conventions.

FIXES #70

Signed-off-by: Adityarya11 <arya050411@gmail.com>
Copilot AI review requested due to automatic review settings March 28, 2026 15:14
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses legend overlap in Matplotlib charts by relocating legends to a dedicated right-side margin so plotted data and axis labels aren’t obscured (fixes #70).

Changes:

  • Updated plot_multiline (line charts) to place the legend outside the plot area on the right and reserve layout space via layout_rect.
  • Updated plot_stacked_bar (stacked bar charts) to place the legend outside the plot area on the right and reserve layout space via layout_rect.
  • Added an updated generated chart image under outputs/charts/....

Reviewed changes

Copilot reviewed 2 out of 8 changed files in this pull request and generated 5 comments.

File Description
src/hiero_analytics/plotting/lines.py Moves multiline legend to the right sidebar and reserves space using layout_rect.
src/hiero_analytics/plotting/bars.py Moves stacked-bar legend to the right sidebar and reserves space using layout_rect (including handling for legend_inside_bottom_right).
outputs/charts/org/hiero-ledger/gfi_pipeline.png Adds a regenerated chart artifact reflecting the new legend layout.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

@MonaaEid MonaaEid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Copy Markdown
Contributor

@danielmarv danielmarv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Because it gives visibility to the X axis scales and labling

Copy link
Copy Markdown
Contributor

@exploreriii exploreriii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree this is a good solution, because this narrows all the charts windows significantly in some cases

We could instead just put the legend below the x axis? or at least choose the optimal location by type of chart

@Adityarya11
Copy link
Copy Markdown
Contributor Author

I disagree this is a good solution, because this narrows all the charts windows significantly in some cases

We could instead just put the legend below the x axis? or at least choose the optimal location by type of chart

I have added backward compatibility, bottom legend, and the right legend only when there are too many items in the legend
other wise in some the legends are inside the charts and others have in bottom

Signed-off-by: Adityarya11 <arya050411@gmail.com>
Copy link
Copy Markdown
Contributor

@exploreriii exploreriii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we will have to think in the future how to handle the different needs for charts, as this is hard-guessing the placement. But much improved, thank you!

@exploreriii
Copy link
Copy Markdown
Contributor

@MonaaEid request re-review

@exploreriii exploreriii mentioned this pull request Mar 31, 2026
@exploreriii exploreriii merged commit 58f70ec into hiero-hackers:main Apr 1, 2026
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

Successfully merging this pull request may close these issues.

5 participants