Skip to content
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

Resolve 'text ignores rotational part of transformation' (#698) #15532

Conversation

immaxchen
Copy link
Contributor

@immaxchen immaxchen commented Oct 26, 2019

PR Summary

Considering the text rotation don't get transformed may be a desired behavior
(such as text for annotation to some data point). Here made it as an option to
control whether or not the text rotation get transformed.

text = plt.text(0, 0, 'hello', size=50, transform=R + ax.transData)

Screenshot from 2019-10-27 00-02-16-small

  • behavior in this patch:
text = plt.text(0, 0, 'hello', size=50, transform=R + ax.transData, 
                rotation_mode='anchor', rotation_transformed=True)

Screenshot from 2019-10-26 23-57-37-small

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

lib/matplotlib/text.py Outdated Show resolved Hide resolved
lib/matplotlib/text.py Outdated Show resolved Hide resolved
@timhoffm timhoffm added this to the v3.3.0 milestone Nov 17, 2019
@anntzer
Copy link
Contributor

anntzer commented Dec 7, 2019

Do we really want to expose a control knob as to whether rotation affects text? It would seem(??) like we generally do want it to affect text. Perhaps just have that for a transition period and warn if the transform has nonzero rotation but that rotation was getting ignored and won't be ignored after the transition?
(Not really sure of the best strategy here.)

@timhoffm
Copy link
Member

timhoffm commented Dec 7, 2019

Do we really want to expose a control knob as to whether rotation affects text? It would seem(??) like we generally do want it to affect text.

Not sure how it works internally, but wouldn't that rotate tick labels on a polar plot?

@anntzer
Copy link
Contributor

anntzer commented Dec 7, 2019

Ah, fair point.

@anntzer
Copy link
Contributor

anntzer commented Dec 7, 2019

Does this need a whatsnew note?

@immaxchen
Copy link
Contributor Author

Sure, no problem.

@QuLogic
Copy link
Member

QuLogic commented May 8, 2020

This needs a rebase to resolve conflicts.

@tacaswell tacaswell force-pushed the text-rotation-can-get-transformed-too-gh698 branch from 8e28935 to 800e810 Compare June 2, 2020 03:47
@tacaswell
Copy link
Member

@immaxchen I took the liberty of rebasing and force-pushing for you, I hope you do not mind.

@immaxchen
Copy link
Contributor Author

sure & thank you for the help.

@anntzer
Copy link
Contributor

anntzer commented Jun 6, 2020

To what extent would it be possible to make this also improve the situation with #17005?

@immaxchen
Copy link
Contributor Author

#17005 can be easily fixed this way, not sure if you like it though.

@tacaswell tacaswell modified the milestones: v3.3.0, v3.4.0 Jun 8, 2020
if self.get_transform_rotates_text():
angle = get_rotation(self._rotation)
x, y = self.get_unitless_position()
return self.get_transform().transform_angles([angle, ], [[x, y]])
Copy link
Member

Choose a reason for hiding this comment

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

This seems like it has type instability?

Copy link
Contributor Author

@immaxchen immaxchen Jun 10, 2020

Choose a reason for hiding this comment

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

I fixed the inconsistent type, but don't know why CI keeps failing.. i think i didn't do anything about docs in the new commit
(edit) it turns out that a rebase is needed to pass CI ..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tacaswell resolved, please have a look

@immaxchen immaxchen force-pushed the text-rotation-can-get-transformed-too-gh698 branch from dac1bb2 to 230f0fb Compare June 10, 2020 13:50
@immaxchen immaxchen force-pushed the text-rotation-can-get-transformed-too-gh698 branch from 68ed931 to cee60e7 Compare June 18, 2020 13:31
…lib#698)

Considering the text rotation don't get transformed may by a desired behavior
(such as text for annotation to some data point). Here made it as an option to
control whether or not the text rotation get transformed.
@immaxchen immaxchen force-pushed the text-rotation-can-get-transformed-too-gh698 branch from 44df434 to 68cfd75 Compare June 19, 2020 16:09
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.

I'm not sure why Travis didn't report here, but it's green.

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.

None yet

6 participants