Skip to content

Support non-integer bases in mathtext sqrt #19186

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

Conversation

timhoffm
Copy link
Member

PR Summary

Fixes #8665 by using the code snippet proposed there #8665 (comment).

The test images show that it works.

I'm a bit hesitant to add 72kB of images just for testing this. I would also be ok to strip the test again and commit without tests. It's rather unlikely that we'll touch the base part of sqrt again and break it.

@timhoffm timhoffm force-pushed the noninteger-base-sqrt-mathtext branch from 1d71821 to 96a2601 Compare December 29, 2020 01:29
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’d much rather have the tests. 72kb doesn’t seem worth worrying about.

@anntzer
Copy link
Contributor

anntzer commented Dec 29, 2020

Actually one possibility would be to only test in svg and using rcParams["svg.fonttype"] = "none", which avoids embedding the font, and therefore results in a very small (1.7kb) file. (Another 30% could be chopped off if you could get rid of the metadata block...)

In order to render the result correctly, inkscape needs to know the path to Matplotlib fonts. This can either be done by adding "Additional font directories" in the Inkscape GUI under preferences/tools/text, or (I guess that'll be the practical solution for us, which should be reasonably easily automatable) by copying the Matplotlib fonts to e.g. /tmp/foo/inkscape/fonts and running inkscape with XDG_CONFIG_HOME=/tmp/foo.

I think the savings may be worth it? Especially as this should allow us to work much more regularly on adding mathtext improvements without being restricted by the baseline images problem at all.

@aitikgupta
Copy link
Contributor

Can we not merge the new tests with the old ones?
(Might not affect the overall size, but could reduce the need to add new images for every new supported symbol?)
@anntzer said something in a similar line of thought here #18916 (comment)

Copy link
Contributor

@anntzer anntzer left a comment

Choose a reason for hiding this comment

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

Let's decide on #19201 first.

@anntzer
Copy link
Contributor

anntzer commented Jan 16, 2021

Superseded by #19261.

@anntzer anntzer closed this Jan 16, 2021
@timhoffm timhoffm deleted the noninteger-base-sqrt-mathtext branch January 16, 2021 23:02
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.

Noninteger Bases in mathtext sqrt
4 participants