Skip to content

BUG: % crashes saving figure with tex enabled #6886

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

Closed
wants to merge 1 commit into from

Conversation

madphysicist
Copy link
Contributor

This PR changes the default symbol used by mpl.ticker.PercentFormatter if mpl.rcParams['text.usetex'] == True. There is room for improvement, as evidenced by this question: http://stackoverflow.com/q/38731201/2988730.

Tests coming soon.

@Kojoley
Copy link
Member

Kojoley commented Sep 8, 2016

I cannot reproduce savefig crash, but you have found a more deeper problem. % in latex is used for comments, so with usetex=True it should not crash, but percent symbol would not appear too; and the issue is not only in PercentFormatter, but in the test suite too.


usetex=True image

usetex=False image

import matplotlib
import matplotlib.pyplot as plt
from matplotlib.ticker import PercentFormatter

matplotlib.rc('text', usetex=True)

x, y = [1, 2, 3], [64, 128, 256]

fig, ax = plt.subplots()
ax.yaxis.set_major_formatter(PercentFormatter(xmax=512))
ax.bar(x, y)
fig.suptitle('usetex=True')
#fig.savefig('test_usetex-percentformatter.png')
plt.show()

@tacaswell tacaswell added this to the 2.1 (next point release) milestone Sep 8, 2016
@WeatherGod
Copy link
Member

I could have sworn we have code that escapes all percent signs before
passing to latex, don't we?

On Thu, Sep 8, 2016 at 11:15 AM, Nikita Kniazev notifications@github.com
wrote:

I cannot reproduce savefig crash, but you have found a more deeper
problem. % in latex is used for comments, so with usetex=True it should
not crash, but percent symbol would not appear too; and the issue is not

only in PercentFormatter, but in the test suite too.

[image: usetex=True image]
https://cloud.githubusercontent.com/assets/2743474/18354317/c0d5b2b6-75ed-11e6-9318-bfd73847074b.png

[image: usetex=False image]
https://cloud.githubusercontent.com/assets/2743474/18354341/d409b8b4-75ed-11e6-8b8f-b5a56acde7c6.png

import matplotlibimport matplotlib.pyplot as pltfrom matplotlib.ticker import PercentFormatter

matplotlib.rc('text', usetex=True)

x, y = [1, 2, 3], [64, 128, 256]

fig, ax = plt.subplots()
ax.yaxis.set_major_formatter(PercentFormatter(xmax=512))
ax.bar(x, y)
fig.suptitle('usetex=True')#fig.savefig('test_usetex-percentformatter.png')
plt.show()


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#6886 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AARy-ISVRGajISmHkAQurd9rx_-ScCZ1ks5qoCapgaJpZM4JbGY7
.

@madphysicist
Copy link
Contributor Author

Nope. As my SO question states, I can't even find a list of characters that need to be escaped anywhere in matplotlib.

@madphysicist
Copy link
Contributor Author

@Kojoley The savefig issue is caused by the version of LaTeX on my machine, which barfs on non-escaped percent symbols. It is basically the exact same issue you are having, but with a different backend.

Another problem here is that symbols should not be escaped in __init__ but rather in __call__. In theory, the value of rcParams['text.usetex'] can change between those two calls. Either way, we probably need a latex-escaping routine somewhere.

In the meantime, you can always pass in your own custom symbol (like r'\%') if you are planning on using PercentFormatter on a latex-enabled axis.

@Kojoley
Copy link
Member

Kojoley commented Sep 8, 2016

I could have sworn we have code that escapes all percent signs before
passing to latex, don't we?

I have crawled all the way with a debugger and there was only fix_minus call related to escaping.
image

Here you can see a rendering cache. PercentFormatter strings are not escaped.

@madphysicist
Copy link
Contributor Author

This PR has been superseded and significantly improved on by #7965. Closing.

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.

6 participants