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

Axis labels should respect the theme (#142) and fix #96 whilst I'm at it.#149

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

Merged

Conversation

samcunliffe
Copy link
Collaborator

@samcunliffesamcunliffe commentedJun 1, 2023•
edited
Loading

Fixes#142 by reapplying the colour schemebeforecanvas.draw inNapariMPLWidget._draw (cheers@dstansby!).

Since fixing the colour for axis labels means I need to regenerate the reference scatter plots, I thought I might as wellfix#96 by setting theviewer.theme for every test that is marked@pytest.mark.mpl_image_compare. It's not a fix via a nice global stylesheet setting but... you can now read the axes on the baseline figures. (Which is maybe nice??)

Testing:

  • Asomewhat trustworthy TDD reproduction is added intotest_theme.

Notes for reviewers:

Please double-check for me that a)main really does have#142, and that b) this really does solve it. I have had some strangeness that I am currently blaming onpip install without-e.

@samcunliffesamcunliffeforce-pushed the142-titles-not-respecting-theme branch froma7bdda2 to03778c7CompareJune 2, 2023 10:33
@samcunliffesamcunliffeforce-pushed the142-titles-not-respecting-theme branch from359a9fe to2413b53CompareJune 2, 2023 11:21
@samcunliffesamcunliffe changed the titleFailing test that demonstrates #142Axis labels should respect the theme (#142) and fix #96 whilst I'm at it.Jun 2, 2023
@samcunliffesamcunliffe added the BugSomething isn't working labelJun 2, 2023
@samcunliffesamcunliffe marked this pull request as ready for reviewJune 2, 2023 11:33
Copy link
Collaborator

@ruaridhgruaridhg left a comment

Choose a reason for hiding this comment

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

The axis labels were black when running napari frommain, but running from this branch meant that they were white in dark mode and black in light mode so yay!

@ruaridhgruaridhg added this pull request to the merge queueJun 2, 2023
@github-merge-queuegithub-merge-queuebot removed this pull request from the merge queue due to a conflict with the base branchJun 2, 2023
@samcunliffesamcunliffeforce-pushed the142-titles-not-respecting-theme branch from2413b53 to4af1c3fCompareJune 2, 2023 15:08
@ruaridhg
Copy link
Collaborator

ruaridhg commentedJun 2, 2023•
edited
Loading

Works locally, but tests are failing on tox because the existing baseline images are using the default theme so adding the them causes a difference. Can figure out why next week.
image

Not a nice global fix, but by setting the test fixture viewer's theme tolight, napari-matplotlib will give axes dark grey axes, labels, and ticks.
@samcunliffesamcunliffeforce-pushed the142-titles-not-respecting-theme branch from4af1c3f to21b5823CompareJune 4, 2023 20:31
@samcunliffe
Copy link
CollaboratorAuthor

Works locally, but tests are failing on tox because the existing baseline images are using the default theme so adding the them causes a difference. Can figure out why next week.

I was replacing the plots intests/baseline but didn't see there are also some baseline plots intests/scatter/baseline.

Do I need to get into the habit of runningpytest with--mpl-generate-path pointing to two directories (I guess with a-k to separate the plots into the scatter subdir?

@ruaridhg
Copy link
Collaborator

Works locally, but tests are failing on tox because the existing baseline images are using the default theme so adding the them causes a difference. Can figure out why next week.

I was replacing the plots intests/baseline but didn't see there are also some baseline plots intests/scatter/baseline.

Do I need to get into the habit of runningpytest with--mpl-generate-path pointing to two directories (I guess with a-k to separate the plots into the scatter subdir?

Ah yes, that was a recent change by David. I've been runningpytest with--mpl-generate-path twice with the different directories, but yes pointing to them both in the same command would be more efficient

samcunliffe reacted with thumbs up emoji

@ruaridhgruaridhgenabled auto-mergeJune 5, 2023 08:09
Copy link
Collaborator

@ruaridhgruaridhg left a comment

Choose a reason for hiding this comment

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

LGTM

@ruaridhgruaridhg added this pull request to the merge queueJun 5, 2023
Merged via the queue intomatplotlib:main with commit4a9a4daJun 5, 2023
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@ruaridhgruaridhgruaridhg approved these changes

@dstansbydstansbyAwaiting requested review from dstansby

Assignees
No one assigned
Labels
BugSomething isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Axis titles are always black. Axes text to show up in the test figure
3 participants
@samcunliffe@ruaridhg@dstansby
[8]ページ先頭
©2009-2025 Movatter.jp