Clean up and move text rotation example #8066

Merged
merged 4 commits into from Feb 25, 2017

Conversation

Projects
None yet
4 participants
Contributor

dstansby commented Feb 12, 2017

No description provided.

dstansby added some commits Feb 12, 2017

@dstansby dstansby Clean up text_rotation example 41b7d7f
@dstansby dstansby Move text_rotation example
f19839e
+Text rotation demonstration
+===========================
+
+The way matplotlib does text layout is counter-intuitive to some, so
@phobson

phobson Feb 13, 2017

Member

Maybe we could be a little gentler about this. Something like, "matplotlib takes a different approach to text layout..." (maybe?)

@QuLogic

QuLogic Feb 14, 2017

Member

matplotlib -> Matplotlib

+aligned by it's bounding box (the rectangular box that surrounds the
+ink rectangle). The order of operations is rotation then
+alignment, rather than alignment then rotation. Basically, the text
+is centered at your x,y location, rotated around this point, and then
@phobson

phobson Feb 13, 2017

Member

is this true with e.g., rotation_mode='anchor' and ha='right'?

@dstansby

dstansby Feb 14, 2017

Contributor

I think probably not.

It might be worth having an example for the default rotation mode (this example), and an example for the 'anchor' rotation mode (to replace http://matplotlib.org/devdocs/examples/pylab_examples/demo_text_rotation_mode.html)

+ ax.text(4.5, 0.5, 'text -45', props, rotation=-45)
+ ax.set_yticks([0, .5, 1])
+ ax.set_xlim(0, 5)
+ ax.grid(True)
@phobson

phobson Feb 13, 2017

Member

Would love to see some of the different rotation modes in this example similar to this: http://matplotlib.org/devdocs/examples/pylab_examples/demo_text_rotation_mode.html

+Text rotation demonstration
+===========================
+
+The way matplotlib does text layout is counter-intuitive to some, so
@QuLogic

QuLogic Feb 14, 2017

Member

matplotlib -> Matplotlib

+
+The way matplotlib does text layout is counter-intuitive to some, so
+this example is designed to make it a little clearer. The text is
+aligned by it's bounding box (the rectangular box that surrounds the
@QuLogic

QuLogic Feb 14, 2017

Member

it's -> its

dstansby added some commits Feb 14, 2017

@dstansby dstansby Scatter text anchor points 6a13691
@dstansby dstansby Tidy up docstring
3c19565
Contributor

dstansby commented Feb 14, 2017

I've tidied up the docstring, and also scattered the text anchor points which I think makes the demo easier to understand.

Does anyone have any thoughts on leaving this as a demo of the default text rotation demonstration, and replacing http://matplotlib.org/devdocs/examples/pylab_examples/demo_text_rotation_mode.html with a demo of the 'anchor' mode of text rotation?

NelleV added this to the 2.1 (next point release) milestone Feb 16, 2017

@NelleV

NelleV approved these changes Feb 16, 2017

LGTM 👍

Contributor

NelleV commented Feb 16, 2017

I would just remove pylab_examples/demo_text_rotation_mode.py.

NelleV changed the title from Clean up and move text rotation example to [MRG+1] Clean up and move text rotation example Feb 16, 2017

Contributor

NelleV commented Feb 16, 2017

I gave my approval, but it'd be nice before we merged this one if you could do a small summary of all related examples in the gallery (ie, text rotation examples) and to merge and delete as many as possible.

Contributor

NelleV commented Feb 23, 2017

Can someone else review this? I think this patch is ready to be merged, but it requires an extra review.

+import numpy as np
+
+
+def addtext(ax, props):
@QuLogic

QuLogic Feb 24, 2017

Member

This function does more than just add text.

+
+
+def addtext(ax, props):
+ ax.text(0.5, 0.5, 'text 0', props, rotation=0)
@QuLogic

QuLogic Feb 24, 2017

Member

How about putting this in the loop too? for i, angle in enumerate([0, 45, 135, 225, -45]):

+fig, axs = plt.subplots(2, 1)
+
+addtext(axs[0], {'ha': 'center', 'va': 'center', 'bbox': bbox})
+axs[0].set_xticks(np.arange(0, 5.1, 0.5), [])
@QuLogic

QuLogic Feb 24, 2017

Member

This could be done in the (suitably-renamed) addtext.

Contributor

NelleV commented Feb 24, 2017

@QuLogic This PR modifies the docstring. The code was there before and @dstansby did not modify it.
I don't think we should be asking contributors to clean up the mess that was there before, in particular in the case of documentation focused PR. Else, our documentation is just never going to improve.

Member

QuLogic commented Feb 24, 2017

Hmm, good point, but @dstansby did modify it or it would have shown as a single move, not two separate files. And because it's @dstansby and not a new contributor, I know he can handle it ;)

Contributor

NelleV commented Feb 24, 2017

Only the docstring was modified, but that was enough for github to consider this not as a move, but as a deletion + modification..
It is just that in general, I think we are asking too much for our documentation patches: many people just give up on improving documentation on Matplotlib, as it is often harder than contributing code.

@dstansby Let us know if you'd like to fix the core content of the example, or whether you'd rather have this merged as-is.

Contributor

dstansby commented Feb 25, 2017

Hello, I would prefer this to be merged as is - I only intended it to be a MEP-12 clean and a move, and if anyone wants to change what the example does they can always open another PR.

@NelleV NelleV merged commit 65201e0 into matplotlib:master Feb 25, 2017

5 checks passed

codecov/patch Coverage not affected when comparing 3d99e43...3c19565
Details
codecov/project/library 62.28% remains the same compared to 3d99e43
Details
codecov/project/tests 97.97% (target 97.7%)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

QuLogic changed the title from [MRG+1] Clean up and move text rotation example to Clean up and move text rotation example Feb 26, 2017

dstansby deleted the dstansby:doc-text-rotation branch Apr 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment