Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

tests: extend QtAgg figureoptions coverage (follow-up to #30128)#30587

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

Open
YQ157 wants to merge9 commits intomatplotlib:main
base:main
Choose a base branch
Loading
fromYQ157:fix-legend-test-coverage

Conversation

YQ157
Copy link

@YQ157YQ157 commentedSep 19, 2025•
edited
Loading

This PR extends the test suite for the QtAgg figureoptions dialog,
addressing coverage gaps reported on PR#30128 (Codecov) and related to issue#11109.

What’s included:

  • Parameterized legend scenarios (legend present/absent, visible/invisible)
  • Date axis handling
  • Lines labeled "nolegend" (skipped from legend)
  • Mappables without array data
  • Preserving existing legend when regenerate=False
  • Stronger assertions for redraw and navigation stack updates

The underlying implementation was originally proposed in#30128.
This PR only adds tests (with minimal supporting changes), to ensure more complete coverage
and to make it easier for maintainers to review and merge the fix.

Refs#30128,#11109

ℹ️ Note: This is my first open-source contribution.
It directly addresses the same problem described in issue#11109, which was marked as a good first issue.

@YQ157YQ157force-pushed thefix-legend-test-coverage branch 2 times, most recently from85ae698 toa1434f3CompareSeptember 19, 2025 16:19
@YQ157YQ157 closed thisSep 19, 2025
@YQ157YQ157 reopened thisSep 19, 2025
@YQ157
Copy link
Author

Onlyubuntu-24.04-arm (Python 3.12) failed withtest_webagg timeout. Other platforms passed. Could a maintainer please re-run the ARM job?

@YQ157
Copy link
Author

YQ157 commentedSep 20, 2025•
edited
Loading

Hi maintainers,

The only failing check is "Python 3.12 on ubuntu-24.04-arm", where test_webagg appears to time out after 120 s.
This seems to be an environment issue on ARM64 + Python 3.12 runners rather than caused by this PR.
Could you please override this job or let me know if I should skip the test?
Thanks for your help!

@YQ157
Copy link
Author

All checks have passed ✅

This is not only my first contribution to Matplotlib, but also my first contribution to an open-source project.
I’m really excited to see it ready, and I’m looking forward to your review/merge.

Thanks a lot!

YQ157 reacted with eyes emoji

@QuLogic
Copy link
Member

I'm a bit confused by the PR title; there are more files changed here than just tests. Also, please don't reformat entire files; that makes it difficult to see what your actual changes are.

@YQ157YQ157 changed the titletests: broaden QtAgg figureoptions test coveragetests: extend QtAgg figureoptions coverage (follow-up to #30128)Sep 23, 2025
@YQ157
Copy link
Author

I'm a bit confused by the PR title; there are more files changed here than just tests. Also, please don't reformat entire files; that makes it difficult to see what your actual changes are.

Thanks for the feedback!

I’ve reverted the accidental reformatting, so the diff should now be clean.
To clarify: the changes infigureoptions.py come from PR#30128 and are not new in this PR.
My contribution here is only the additions intest_backend_qt.py, which extend coverage for the QtAgg figureoptions dialog.
There are no new implementation changes beyond what#30128 proposed.

@rcomer
Copy link
Member

If this PR is meant to go on top of#30128 then I think we need to bring that to a conclusion before reviewing this one.

Add a second callback invocation with a 4-tuple mappable(label, cmap, low, high) to exercise the `elif len(...) == 4`branch in `figure_edit`. Use fresh copies of the input blockssince the callback mutates them.
@YQ157
Copy link
Author

If this PR is meant to go on top of#30128 then I think we need to bring that to a conclusion before reviewing this one.

Thanks for the clarification!

You are correct — this PR is based on#30128, and its purpose is to add the missing tests needed for its CI checks to pass.
I realize this creates a dependency loop, so I’m happy to follow whichever workflow works best for the maintainers to resolve both PRs.

Please let me know how I can help.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Projects
Status: Needs review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@YQ157@QuLogic@rcomer@ritvi-alagusankar
[8]ページ先頭
©2009-2025 Movatter.jp