Skip to content

FIX: force blended transforms with data to be in data space#17017

Merged
jklymak merged 2 commits intomatplotlib:masterfrom
jklymak:fix-blended-transform
Apr 3, 2020
Merged

FIX: force blended transforms with data to be in data space#17017
jklymak merged 2 commits intomatplotlib:masterfrom
jklymak:fix-blended-transform

Conversation

@jklymak
Copy link
Copy Markdown
Member

@jklymak jklymak commented Apr 3, 2020 •
edited
Loading

PR Summary

Closes #17016
(Maybe?)

This is a very naive approach. Needs tests so we don't break it again. Not sure what the correct thing to do for blended transforms is

import numpy as np
import matplotlib as mpl
import matplotlib.transforms as tx
from matplotlib.collections import LineCollection
import matplotlib.pyplot as plt

a = [1, 2, 4]
height = .2

xy_pairs = np.column_stack([np.repeat(a, 2), np.tile([0, height], len(a))])
line_segs = xy_pairs.reshape([len(a), 2, 2])

f, ax = plt.subplots()
trans = tx.blended_transform_factory(ax.transData, ax.transAxes)
ax.add_collection(LineCollection(line_segs, transform=trans))
ax.autoscale_view(scalex=True, scaley=False)

Now returns

fix

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@jklymak jklymak added this to the v3.2.2 milestone Apr 3, 2020
@jklymak jklymak requested a review from anntzer April 3, 2020 18:14
This was referenced Apr 3, 2020
Closed
Closed
@jklymak jklymak force-pushed the fix-blended-transform branch from 593e41f to bceaab2 Compare April 3, 2020 18:57
@anntzer
Copy link
Copy Markdown
Contributor

anntzer commented Apr 3, 2020

I think the correct thing is "just" to autoscale only along dimensions that contains_branch_separately(transData), but not others? Not saying it's so easy :/

anntzer
anntzer approved these changes Apr 3, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@anntzer anntzer left a comment

Choose a reason for hiding this comment

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

Basic approach lgtm, perhaps needs to be noted in whatsnew?

@tacaswell
Copy link
Copy Markdown
Member

tacaswell commented Apr 3, 2020

Not sure this needs a whats_new as it is fixing a regression.

tacaswell
tacaswell approved these changes Apr 3, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@tacaswell tacaswell left a comment

Choose a reason for hiding this comment

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

Anyone can merge on CI green.

@tacaswell tacaswell added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Apr 3, 2020
@jklymak jklymak changed the title FIX: force bended transforms with data to be in data space FIX: force blended transforms with data to be in data space Apr 3, 2020
@jklymak jklymak force-pushed the fix-blended-transform branch from bceaab2 to ba075b1 Compare April 3, 2020 20:18
@jklymak
Copy link
Copy Markdown
Member Author

jklymak commented Apr 3, 2020

Fixed typos in commit messages, otherwise same PR as approved

@jklymak
Copy link
Copy Markdown
Member Author

jklymak commented Apr 3, 2020

I'll self merge to hopefully make it easier for @moorepants to check if it also fixes #17004

@jklymak jklymak merged commit 98dff87 into matplotlib:master Apr 3, 2020
@lumberbot-app
Copy link
Copy Markdown

lumberbot-app Bot commented Apr 3, 2020

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
$ git checkout v3.2.x
$ git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
$ git cherry-pick -m1 98dff875253b23f7e94611b4e2118a239792dbe8
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
$ git commit -am 'Backport PR #17017: FIX: force blended transforms with data to be in data space'
  1. Push to a named branch :
git push YOURFORK v3.2.x:auto-backport-of-pr-17017-on-v3.2.x
  1. Create a PR against branch v3.2.x, I would have named this PR:

"Backport PR #17017 on branch v3.2.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free to suggest an improvement.

@ImportanceOfBeingErnest
Copy link
Copy Markdown
Member

ImportanceOfBeingErnest commented Apr 3, 2020

No, I think #17004 is caused by #13593, which apparently was a deliberate change.

jklymak added a commit to jklymak/matplotlib that referenced this pull request Apr 3, 2020
@jklymak
Merge pull request matplotlib#17017 from jklymak/fix-blended-transform
ffeeeef 
FIX: force blended transforms with data to be in data space
@jklymak jklymak mentioned this pull request Apr 3, 2020
Merged
6 tasks
jklymak added a commit to jklymak/matplotlib that referenced this pull request Apr 3, 2020
@jklymak
Merge pull request matplotlib#17017 from jklymak/fix-blended-transform
cfff1bc 
FIX: force blended transforms with data to be in data space
QuLogic added a commit that referenced this pull request Apr 3, 2020
@QuLogic
Merge pull request #17020 from jklymak/backport17017
fd09e5c 
Merge pull request #17017 from jklymak/fix-blended-transform
@jklymak jklymak deleted the fix-blended-transform branch April 3, 2020 23:37
@anntzer anntzer mentioned this pull request Oct 22, 2020
Closed
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tacaswell tacaswell tacaswell approved these changes

@anntzer anntzer anntzer approved these changes

Assignees

No one assigned

Labels

Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.

Projects

None yet

Milestone

v3.2.2

Development

Successfully merging this pull request may close these issues.

Issues with autoscaling and transforms with 3.2+

5 participants

@jklymak @anntzer @tacaswell @ImportanceOfBeingErnest @QuLogic