Skip to content

FIX: layout for mixed descent multiline text objects - #11499

Merged
timhoffm merged 1 commit into
matplotlib:masterfrom
jklymak:fix-mixed-descent-multiline
Sep 9, 2018
Merged

timhoffm merged 1 commit into
matplotlib:masterfrom
jklymak:fix-mixed-descent-multiline

Conversation

@jklymak

@jklymak jklymak commented Jun 24, 2018 •

Copy link
Copy Markdown
Member

PR Summary

Closes #11498 partially closes #11468

WIP: this'll fail some tests that depended on the previous bad behaviour...

EDIT This indeed failed some tests, but I'd pretty strongly argue the tests were broken and this PR makes them correct...

Test:

import matplotlib.pyplot as plt
import matplotlib.patches as mpatches

fig, ax = plt.subplots(constrained_layout=False)

ax.set_xlim([0, 1.4])
ax.set_ylim([0, 2])
ax.axhline(0.5, color='C2', linewidth=0.3)
sts = ['Line', '2 Lines g\n 2nd Line g', '$\sum_i x $', 'hi $\sum_i x $\ntest', 'test\n $\sum_i x $', '$\sum_i x $\n $\sum_i x $']

for nn, st in enumerate(sts):
    tt = ax.text(0.2 * nn + 0.1, 0.5, st, horizontalalignment='center',
        verticalalignment='bottom')
    bb = tt.get_window_extent(fig.canvas.get_renderer())
    bbt = bb.inverse_transformed(ax.transAxes)
    r = mpatches.Rectangle((0,0), 1, 1, clip_on=False, transform=ax.transAxes)
    r.set_bounds(bbt.bounds)
    ax.add_patch(r)
ax.text(1.2, 0.5, 'Bottom align', color='C2')


if 1:

    ax.axhline(1.3, color='C2', linewidth=0.3)
    for nn, st in enumerate(sts):
        tt = ax.text(0.2 * nn + 0.1, 1.3, st, horizontalalignment='center',
            verticalalignment='top')
        bb = tt.get_window_extent(fig.canvas.get_renderer())
        bbt = bb.inverse_transformed(ax.transAxes)
        r = mpatches.Rectangle((0,0), 1, 1, clip_on=False, transform=ax.transAxes)
        r.set_bounds(bbt.bounds)
        ax.add_patch(r)
    ax.text(1.2, 1.3, 'Top align', color='C2')


    ax.axhline(1.8, color='C2', linewidth=0.3)
    for nn, st in enumerate(sts):
        tt = ax.text(0.2 * nn + 0.1, 1.8, st, horizontalalignment='center',
            verticalalignment='baseline')
        bb = tt.get_window_extent(fig.canvas.get_renderer())
        bbt = bb.inverse_transformed(ax.transAxes)
        r = mpatches.Rectangle((0,0), 1, 1, clip_on=False, transform=ax.transAxes)
        r.set_bounds(bbt.bounds)
        ax.add_patch(r)
    ax.text(1.2, 1.8, 'Baseline align', color='C2')


    ax.axhline(0.1, color='C2', linewidth=0.3)

    for nn, st in enumerate(sts):
        tt = ax.text(0.2 * nn + 0.1, 0.1, st, horizontalalignment='center',
            verticalalignment='bottom', rotation=20)
        bb = tt.get_window_extent(fig.canvas.get_renderer())
        bbt = bb.inverse_transformed(ax.transAxes)
        r = mpatches.Rectangle((0,0), 1, 1, clip_on=False, transform=ax.transAxes)
        r.set_bounds(bbt.bounds)
        ax.add_patch(r)
    ax.text(1.2, 0.1, 'Bot align, rot20', color='C2')


bb = tt.get_window_extent(fig.canvas.get_renderer())
bbt = bb.inverse_transformed(ax.transAxes)
r = mpatches.Rectangle((0,0), 1, 1, clip_on=False, transform=ax.transAxes)
r.set_bounds(bbt.bounds)
ax.add_patch(r)

fig.savefig('Old.png')

plt.show()

Before:

old

With PR:

new

PR Checklist

  • FIX baseline alignment for multiline
  • Add blue boxes to test...
  • Has Pytest style unit tests
  • Code is PEP 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 commented Aug 16, 2018

Copy link
Copy Markdown
Member Author

Squashed, and added boxes around the text in the mulitline test per @efiring suggestion...

@efiring efiring left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is making important fixes so it should get in ASAP. I have only some trivial cleanup suggestions, mainly pointing to a few comments that seem both verbose and unclear.

Comment thread lib/matplotlib/tests/test_text.py Outdated
renderer = fig.canvas.get_renderer()

def draw_box(ax, tt):
if 1:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks like this conditional is left over from development.

Comment thread lib/matplotlib/text.py Outdated
ymin = horizLayout[-1][1]
ymax = horizLayout[0][1] + horizLayout[0][3]
# Bounding box definition:
# by defeinition, ymax is 0

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Typo in comment. Also, a single line of comment should be enough here.

Comment thread lib/matplotlib/text.py Outdated
# by defeinition, ymax is 0
ymax = 0
# ymin is the baseline of the last line subtract the descent of the
# last line.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't understand the comment; can you condense to one line, and clarify?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Reworded, though I admit that I don't completely remember what this meant. This code is very convoluted, and could maybe do with a refactor now that we know what its supposed to do. ...

TST: add another multiline test

DOC: fix comments
@jklymak
jklymak force-pushed the fix-mixed-descent-multiline branch from e7420c6 to e3fd995 Compare August 19, 2018 04:03
@jklymak

jklymak commented Aug 29, 2018

Copy link
Copy Markdown
Member Author

Pinging for a second review on this...

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.

Test layout with big descenders and multiple lines inconsistent. Layout managers have problems with titles containing MathText

3 participants