Skip to content

Fix handling of "d" glyph in backend_ps.#19983

Merged
dstansby merged 1 commit intomatplotlib:masterfrom
anntzer:psd
Apr 16, 2021
Merged

Fix handling of "d" glyph in backend_ps.#19983
dstansby merged 1 commit intomatplotlib:masterfrom
anntzer:psd

Conversation

@anntzer
Copy link
Copy Markdown
Contributor

@anntzer anntzer commented Apr 16, 2021

PR Summary

Closes #19979.
A rather "interesting" bug :p

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

@anntzer anntzer added Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. backend: ps labels Apr 16, 2021
@anntzer anntzer added this to the v3.4.2 milestone Apr 16, 2021
@WeatherGod
Copy link
Copy Markdown
Member

Correct me if I am wrong, but wouldn't the same problem happen if someone has a "_d" in their figure?

@anntzer
Copy link
Copy Markdown
Contributor Author

anntzer commented Apr 16, 2021

Actually the problem is if someone has a font with a glyph whose name is _d, which I guess may be technically possible though likely in violation of the glyph naming rules at https://github.com/adobe-type-tools/agl-specification.
Even if we choose to not abbreviate bind def we can still end up running into a wall with really bad glyph names (e.g. a glyph named "bind" or "def"...), which probably don't exist in practice though. So I'd rather just stick with this fix for now, and if this really happens, then we can think of a general glyph renaming facility to just map them all to uuids :/

@WeatherGod
Copy link
Copy Markdown
Member

Fair enough.

@dstansby dstansby merged commit 0c84324 into matplotlib:master Apr 16, 2021
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Apr 16, 2021
@anntzer anntzer deleted the psd branch April 16, 2021 19:55
QuLogic added a commit that referenced this pull request Apr 16, 2021
…983-on-v3.4.x

Backport PR #19983 on branch v3.4.x (Fix handling of "d" glyph in backend_ps.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend: ps Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Blank EPS figures if plot contains 'd'

3 participants