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

Avoid single_path_optimization if offsets are non-zero#16517

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

Closed
dstansby wants to merge2 commits intomatplotlib:masterfromdstansby:avoid-optim

Conversation

dstansby
Copy link
Member

@dstansbydstansby commentedFeb 15, 2020
edited
Loading

This is a stopgap tofix#16461 - I am guessing this is made redundant by#13696, but is a simpler fix and probably easier to get into 3.2.0.

@dstansbydstansby added this to thev3.2.0 milestoneFeb 15, 2020
@anntzer
Copy link
Contributor

Perhaps do a test like given in#16461 but settings x/ylim so that in the correct case the entire patch is out of bounds, but would be drawn at (0, 0) otherwise, and then check_figures_equal() against an empty plot?

dstansby reacted with thumbs up emoji

@dstansby
Copy link
MemberAuthor

That worked well - the test I've added fails on current master but passes here.

jklymak
jklymak previously approved these changesFeb 15, 2020
@jklymakjklymak self-requested a reviewFebruary 15, 2020 17:35
@anntzer
Copy link
Contributor

anntzer commentedFeb 15, 2020
edited
Loading

If we need to change baseline images (especially non-hexbin-related ones) I'd really rather just go with#13696 -- this bug has apparently been present for along time, I'm not convinced it's RC to have it fixed in 3.2. Or if it helps I can make VectorTransform public in#13696; I don't like it but it's not the end of the world.

@dstansby
Copy link
MemberAuthor

Agree with not needing to put this in 3.2. Won't the baseline images have to change anyway at some point though?

@dstansbydstansby modified the milestones:v3.2.0,v3.3.0Feb 15, 2020
@anntzer
Copy link
Contributor

I don't think so. I think (not 100% sure?) here you disabled single_path_optimization when offsets are nonzero, but they really only need to be disabled when offsets are nonzeroand offset_position="data". When offsets are nonzero and offset_position="screen" (which will always be the case after#13696), the old baseline images are still fine.

@dstansby
Copy link
MemberAuthor

Ah yes, true - I can check for offset_position = 'data' here and not worry about the baseline images then.

Surely going through either of the two code paths should give identical results, but it doesn't look like it does because the images have changed. I guess the question is for offset_position = 'screen', which code path is doing the right thing?

@dstansbydstansby reopened thisFeb 15, 2020
@anntzer
Copy link
Contributor

Basically single_path_optimization is slightly "wrong", because it "rounds" the markers to the closest pixel (it's fundamentally the original reason I wrote mplcairo...), see e.g.#7233.
But there are cases, e.g. ticks, where the rounding is actually useful because if you try to draw short ticks antialiased "between" two rows of pixel, they look particularly bad.

dstansby reacted with thumbs up emoji

Co-Authored-By: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
@QuLogic
Copy link
Member

So this is replaced by#13696?

@anntzer
Copy link
Contributor

I think so? feel free to reopen if not.

@QuLogicQuLogic removed this from thev3.3.0 milestoneApr 27, 2020
@dstansbydstansby deleted the avoid-optim branchApril 28, 2020 09:26
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@anntzeranntzeranntzer approved these changes

@timhoffmtimhoffmtimhoffm approved these changes

@jklymakjklymakAwaiting requested review from jklymak

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Hexbin if singular and mincnt used
5 participants
@dstansby@anntzer@QuLogic@jklymak@timhoffm

[8]ページ先頭

©2009-2025 Movatter.jp