10000 Fix Text layout cache lookup. by QuLogic · Pull Request #7420 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]

Skip to content

Fix Text layout cache lookup. #7420

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 8, 2016

Conversation

QuLogic
Copy link
Member
@QuLogic QuLogic commented Nov 7, 2016

Even though Text._get_layout takes a renderer parameter, the cache is looked up by the self._renderer attribute. If an alternate renderer is provided without changing any other properties, the cached layout from the previous renderer is returned.

An alternate renderer is passed by offsetbox.TextArea, which, through its use in legends, causes the legend to shift slightly if figures are saved in different formats.

Fixes #6899.

Even though `Text._get_layout` takes a `renderer` parameter, the cache
is looked up by the `self._renderer` attribute. If an alternate renderer
is provided without changing any other properties, the cached layout
from the previous renderer is returned.

An alternate renderer is passed by `offsetbox.TextArea`, which, through
its use in legends, causes the legend to shift slightly if figures are
saved in different formats.

Fixes matplotlib#6899.
@QuLogic QuLogic added this to the 2.0.1 (next bug fix release) milestone Nov 7, 2016
@QuLogic
Copy link
Member Author
QuLogic commented Nov 7, 2016

A simpler fix is to have offsetbox.TextArea set the Text._renderer property before calling Text._get_layout. However, I figure that the renderer parameter to _get_layout must have been added for a reason (though I'm not sure what), so it should do the right thing.

@QuLogic
Copy link
Member Author
QuLogic commented Nov 7, 2016

When running with @Kojoley's extension randomization from #6899, only matplotlib.testing.decorators.test_tight_layout8.test fails. It doesn't use legends though, so I'm not sure what's up there.

@tacaswell tacaswell changed the title Fix Text layout cache lookup. [MRG+1] Fix Text layout cache lookup. Nov 8, 2016
@tacaswell
Copy link
Member

I think this is better than updating the internal state.

Copy link
Contributor
@dopplershift dopplershift left a comment

Choose a reason for hiding this comment

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

Looks good to me. I agree, passing through the API rather than modifying state is the way to go.

@dopplershift dopplershift changed the title [MRG+1] Fix Text layout cache lookup. [MRG+2] Fix Text layout cache lookup. Nov 8, 2016
@efiring efiring merged commit a3d7af4 into matplotlib:v2.x Nov 8, 2016
@efiring efiring changed the title [MRG+2] Fix Text layout cache lookup. Fix Text layout cache lookup. Nov 8, 2016
@QuLogic QuLogic deleted the fix-text-layout-cache branch November 8, 2016 23:06
@Kojoley
Copy link
Member
Kojoley commented Nov 8, 2016

@QuLogic thanks for fixing this, it really solves the problem with legends. Shame on me, I was close enough, but have not resolved the root issue T_T.

@QuLogic QuLogic modified the milestones: 2.0.1 (next bug fix release), 2.0 (style change major release) Dec 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0