Skip to content

Fix type annotation for Axes.get_legend() to include None #30321

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Jul 17, 2025
Merged

Fix type annotation for Axes.get_legend() to include None #30321

merged 4 commits into from
Jul 17, 2025
+1 −1

Conversation

nrnavaneet
Copy link
Contributor

@nrnavaneet nrnavaneet commented Jul 16, 2025 •
edited
Loading

Summary

Updates the type annotation of Axes.get_legend() to reflect that the method may return None, improving accuracy and static type checking support.

Updates

  • Fixed Axes.set(..., None) to skip unchanged limits
  • Added test in test_axes.py for None behavior

Uses

  • Ensures static type checkers like mypy and IDEs correctly interpret the return type.
  • Prevents false-positive warnings for None checks when using Axes.get_legend()
  • Aligns .pyi type stubs with the actual implementation and docstring.

Tests

  • Added test_get_legend_returns_none_and_legend in test_axes.py
  • Verifies get_legend() returns None initially, then returns legend after creation

closes #30318

@nrnavaneet
Copy link
Contributor Author

@timhoffm, I've submitted the PR as per the approval. Kindly check and lmk if any changes are required.
Happy to update it. Thank you for the opportunity :)

@rcomer
Copy link
Member

rcomer commented Jul 16, 2025

Thanks @nrnavaneet, the typing change seems good but the new test seems unrelated.

@nrnavaneet
Copy link
Contributor Author

nrnavaneet commented Jul 16, 2025 •
edited
Loading

Oh shoot ! Wrong test. I've made the changes.
Thanks for pointing that out !

@rcomer
Copy link
Member

rcomer commented Jul 17, 2025

If you are using AI to help with your contributions, please check everything it produces before submitting it. See also https://matplotlib.org/devdocs/devel/contribute.html#restrictions-on-generative-ai-usage

I would not generally expect to see a test added with a typing change. In this case, I am in two minds whether to keep it. On one hand, test coverage is in general a good thing and this particular test is not expensive. On the other hand, the method in question is so simple that it is hard to see how it could go wrong, and there are existing tests that exercise it within test_legend.py (although they are targeting other functionality).

@nrnavaneet
Copy link
Contributor Author

Sorry !!, Still a new contributor. Im still figuring things out. Shldve checked before committing. My bad , wont happen again. It was simple idk why i made it so complex. If u want u can discard this pr :(.
Ive Read the rules, thanks again for pointing that out; it won't happen again. I love contributing to this repo.

@nrnavaneet
Copy link
Contributor Author

Kindly lmk how u wanna proceed. I respect any decision you make

dstansby
dstansby reviewed Jul 17, 2025
View reviewed changes
Copy link
Member

@dstansby dstansby left a comment

Choose a reason for hiding this comment

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

For a simple typing change/fix, I agree that we don't need to add a test. Please could you remove the test?

@nrnavaneet
Copy link
Contributor Author

Ofcourse! Will do r8 away !

@nrnavaneet
Copy link
Contributor Author

Removed the test as per the suggestion. Thank you :)

@nrnavaneet nrnavaneet requested a review from dstansby July 17, 2025 16:46
@rcomer rcomer added this to the v3.10.4 milestone Jul 17, 2025
@timhoffm timhoffm merged commit 2b75c9e into matplotlib:main Jul 17, 2025
44 checks passed
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Jul 17, 2025
@timhoffm @meeseeksmachine
Backport PR matplotlib#30321: Fix type annotation for Axes.get_legend…
0139aca 
…() to include None
@meeseeksmachine meeseeksmachine mentioned this pull request Jul 17, 2025
Merged
@nrnavaneet nrnavaneet deleted the feature-branch branch July 17, 2025 18:50
dstansby added a commit that referenced this pull request Jul 17, 2025
@dstansby
Merge pull request #30326 from meeseeksmachine/auto-backport-of-pr-30…
8076615 
…321-on-v3.10.x

Backport PR #30321 on branch v3.10.x (Fix type annotation for Axes.get_legend() to include None)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timhoffm timhoffm timhoffm approved these changes

@rcomer rcomer rcomer approved these changes

@dstansby dstansby Awaiting requested review from dstansby

Assignees
No one assigned
Labels
topic: axes topic: typing
Projects
None yet
Milestone
v3.10.4
Development

Successfully merging this pull request may close these issues.

[Bug]: type annotation of Axes.get_legend() misses None
4 participants
@nrnavaneet @rcomer @timhoffm @dstansby