Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork8.1k
Add ability to toggle legend in the figure options dialog box#30128
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Add ability to toggle legend in the figure options dialog box#30128
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join uson gitter for real-time discussion.
For details on testing, writing docs, and our review process, please seethe developer guide
We strive to be a welcoming and open project. Please follow ourCode of Conduct.
@rcomer gentle ping to help review this PR, this will be my first contribution to matplotlib. Please let me know if there are any changes to be made :) |
I’m sorry but I am not currently available for reviewing. I also am not familiar with this part of the library so it is unlikely I will review it when I am available again. |
@ritvi-alagusanka please add some tests for your code. You may wanna look at the tests for similar features as a model |
ritvi-alagusankar commentedJul 1, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@story645@melissawm I have added a test case for this feature. Could you please help review this PR |
YQ157 commentedSep 19, 2025
Hi, I noticed Codecov reported missing coverage on this PR. Since I don’t have push access to the source branch, I opened a follow-up PR to add the missing tests:#30587. This is actually my first open-source contribution — happy to start by helping here! 🎉 Thanks again for the original work! |
Note that CodeCov failing is not necessarily a blocker to getting this PR merged. |
2> Note that CodeCov failing is not necessarily a blocker to getting this PR merged.https://matplotlib.org/devdocs/devel/development_workflow.html#automated-tests @rcomer Could you help review this PR :) |
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.
Overall, I'm sceptical that the matplotlib core library should invest further into configuration GUI. It's a rabbit hole. Also, all needed API is public and stable, so that such GUIs can be developed to arbitrary complexity by third parties; see e.g.https://pylustrator.readthedocs.io/en/latest/.
) | ||
forname,axisinaxis_map.items() | ||
]), | ||
('Legend visible',axes.legend_isnotNone), |
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.
AFAICS this does not work, because it does not cover the case of an existing legend withset_visible(False)
# toggle legend visibility | ||
ifaxes.legend_isnotNone: | ||
axes.legend_.set_visible(legend_visible) |
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.
I have an issue with this:
Let's say, we don't have a legend, then the checkbox "Legend visible" is unchecked.
If the user checks the box, surpisingly nothing will happen since, we don't have a legend.
Either we would need to generate a legend (not so great beacuse we'd need to assume the defaults, equivalent to alegend()
call), or we'd have to deactivate the checkbox if no legend is available (not sure whether that is possible with the generic dialog concept).
Uh oh!
There was an error while loading.Please reload this page.
PR summary
Added the ability to toggle legend in the figure options dialog box, solving issue#11109. Changes in the PR:
PR checklist