Skip to content

Conversation

tpvasconcelos
Copy link
Owner

@tpvasconcelos tpvasconcelos commented Oct 13, 2025

Closes #315


📚 Documentation preview 📚: https://ridgeplot--349.org.readthedocs.build/en/349/

Copy link

⚠️ Missing changelog entry ⚠️

Please add an entry to the changelog in docs/reference/changelog.md for this PR.
The entry should be in the following format:

- Description of the change ({gh-pr}`349`)

If you don't think a changelog entry is necessary for this change, you can add a skip news label to this PR.

Copy link

codecov bot commented Oct 13, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (58e008c) to head (8c0b821).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #349   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           18        19    +1     
  Lines          659       701   +42     
  Branches        85        87    +2     
=========================================
+ Hits           659       701   +42     
Flag Coverage Δ
combined-src 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tpvasconcelos tpvasconcelos requested a review from Copilot October 13, 2025 13:45
@tpvasconcelos
Copy link
Owner Author

@codex Review this PR

Copy link

@Copilot 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 adds legend grouping functionality to ridgeplot, allowing traces with the same labels to share legend entries. The change introduces a new legendgroup parameter to control whether traces sharing the same name should be grouped together in the legend.

  • Adds a legendgroup boolean parameter to control legend entry grouping behavior
  • Refactors trace creation to use a new LegendContext system for managing legend properties
  • Updates examples and documentation to demonstrate the new legend grouping feature

Reviewed Changes

Copilot reviewed 13 out of 21 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/ridgeplot/_ridgeplot.py Adds legendgroup parameter to main ridgeplot function
src/ridgeplot/_figure_factory.py Integrates LegendContextManager to handle legend grouping logic
src/ridgeplot/_obj/legendcontext.py New module defining legend context management classes
src/ridgeplot/_obj/traces/base.py Refactors trace base class to use LegendContext instead of string labels
src/ridgeplot/_obj/traces/bar.py Updates bar trace implementation to use new legend context system
src/ridgeplot/_obj/traces/area.py Updates area trace implementation to use new legend context system
tests/unit/test_ridgeplot.py Adds test for legend grouping functionality
tests/unit/test_figure_factory.py Updates factory tests to include new legendgroup parameter
tests/unit/obj/traces/test_bar.py Updates bar trace tests to use new LegendContext
tests/e2e/artifacts/basic_hist.json Updates test artifact with new showlegend properties
docs/getting_started/getting_started.md Updates documentation examples to use legendgroup feature
cicd_utils/ridgeplot_examples/_lincoln_weather.py Updates example to demonstrate legendgroup usage
cicd_utils/ridgeplot_examples/_lincoln_weather_red_blue.py Updates color mapping to use new label names

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

trace_type: TraceTypesArray | ShallowTraceTypesArray | TraceType | None = None,
labels: LabelsArray | ShallowLabelsArray | None = None,
row_labels: Collection[str] | None | Literal[False] = None,
legendgroup: bool = False, # TODO: document and rename to smth better!
Copy link

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

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

The TODO comment suggests this parameter needs documentation and a better name. The current name 'legendgroup' and the boolean type don't clearly convey that this enables grouping of legend entries by label name.

Copilot uses AI. Check for mistakes.

Comment on lines +53 to +54
# FIXME: This doesn't seem to work as expected
# legendgrouptitle=Legendgrouptitle(text=label),
Copy link

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

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

The commented FIXME indicates that legendgrouptitle functionality is not working as expected. This suggests there may be an issue with the Plotly legend group title feature that should be investigated or documented.

Copilot uses AI. Check for mistakes.

Copy link

Codex Review: Didn't find any major issues. Bravo.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting

@tpvasconcelos
Copy link
Owner Author

@codex pick a better name for the new parameter and document it in the ridgeplot docstring

Copy link

Summary

Testing

  • ⚠️ pytest tests/unit/test_ridgeplot.py::test_traces_share_legend_entries (fails: missing pytest_cov module in environment)

View task →

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.

Legend groups: legend for ridgeplots with multiple traces per line

1 participant