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

"text positioning with transforms" example (figure 2.4) should perhaps normalize dx, dy by 72, not by fig.dpi #79

Closed
anntzer opened this issue Jan 5, 2023 · 7 comments

Comments

@anntzer
Copy link

anntzer commented Jan 5, 2023

The code for figure 2.4 is reproduced below for convenience:

from matplotlib.transforms import ScaleTranslation

fig = plt.figure(figsize=(6, 4))

ax = fig.add_subplot(2, 1, 1)
plt.text(0.1, 0.1, "A", transform=ax.transAxes)

ax = fig.add_subplot(2, 1, 2)
dx, dy = 10/fig.dpi, 10/fig.dpi
offset = ScaledTranslation(dx, dy, fig.dpi_scale_trans)
plt.text(0, 0, "B", transform=ax.transAxes + offset)

plt.show()

I believe the scaling of dx, dy should be a division by 72, not by fig.dpi (more or less as you do in figure 2.5, in fact), as otherwise the appearance of the figure will depend on figure.dpi; e.g.

from matplotlib import pyplot as plt, transforms as tr

size = 10

for dpi in [72, 300]:
    fig = plt.figure(dpi=dpi)

    ax = fig.add_subplot(2, 1, 1)
    dx = dy = size / fig.dpi
    offset = tr.ScaledTranslation(dx, dy, fig.dpi_scale_trans)
    ax.text(0, 0, "A", transform=ax.transAxes + offset, size=size)

    ax = fig.add_subplot(2, 1, 2)
    dx = dy = size / 72
    offset = tr.ScaledTranslation(dx, dy, fig.dpi_scale_trans)
    ax.text(0, 0, "A", transform=ax.transAxes + offset, size=size)

    fig.savefig(f"/tmp/test{dpi}.png")
    fig.savefig(f"/tmp/test{dpi}.pdf")

You can verify that here, the visual appearance of the top axis changes whereas the bottom one doesn't.

In a sense, this is redundant with figure 2.5; but perhaps division-by-72 should really be mentioned first and division-by-fig.dpi is more a side possibility?

@rougier
Copy link
Owner

rougier commented Jan 9, 2023

You're right! Could you make a PR (for both book content and script)?

@anntzer
Copy link
Author

anntzer commented Jan 9, 2023

Can I leave that to you? :) Especially as I think (as noted above) there are some editorial decisions as to how to handle the redundancy with figure 2.5.

@rougier
Copy link
Owner

rougier commented Jan 9, 2023

Sure. Note that it's not really redundant with figure 2.5 since in that case I use a blended transform to mix data/figue coordinate.

@anntzer
Copy link
Author

anntzer commented Feb 12, 2023

Closed by 8d14fc2, afaict. Thanks :)

@anntzer anntzer closed this as completed Feb 12, 2023
@anntzer
Copy link
Author

anntzer commented May 17, 2023

@rougier I recently realized that these examples can be implemented much more simply by using annotate(), which effectively provides a specialized version of ScaledTranslation; see https://github.com/matplotlib/matplotlib/pull/25905/files.
Clearly I think in the book the example with ScaledTranslation should stay at least for pedagogical purposes, but it's up to you whether you want to also mention the "easier" version with annotate().

@rougier
Copy link
Owner

rougier commented May 22, 2023

Oh thanks for the ref, I did not think of it and I confirm it would be worth a mention in the text. Do you want to make a PR?

@anntzer
Copy link
Author

anntzer commented May 22, 2023

I'd rather leave the word-smithing to you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants