FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fixed font prop in text elements in svg figures by not-so-rabh · Pull Request #22541 · matplotlib/matplotlib · GitHub

Repository navigation

fixed font prop in text elements in svg figures - #22541

Closed
not-so-rabh wants to merge 2 commits into
matplotlib:mainfrom
not-so-rabh:fix-font-prop-svg
Closed

not-so-rabh wants to merge 2 commits into
matplotlib:mainfrom
not-so-rabh:fix-font-prop-svg

Conversation

Copy link
Copy Markdown

Fixes #22528

github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a while, please feel free to ping @matplotlib/developers or anyone who has commented on the PR. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join us on gitter for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide

We strive to be a welcoming and open project. Please follow our Code of Conduct.

Comment thread lib/matplotlib/backends/backend_svg.py Outdated

QuLogic left a comment •
edited
Loading

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This seems fine if the original reporter says it works. I cannot reproduce the issue in Chromium.

QuLogic requested a review from anntzer March 2, 2022 01:38

anntzer commented Mar 2, 2022

Copy link
Copy Markdown
Contributor

I don't think this is correct, e.g.

rcParams["svg.fonttype"] = "none"; figtext(.5, .5, "hello", font="DejaVu Sans"); savefig("/tmp/test.svg")

results in

<text style="font: 10px 'DejaVu Sans', DejaVu Sans; text-anchor: start" x="230.4" y="172.8" transform="rotate(-0 230.4 172.8)">hello</text>

where the unquoted DejaVu Sans is likely wrong? (at least wrt. the spec)

anntzer removed their request for review March 2, 2022 08:52
timhoffm added the status: needs clarification Issues that need more information to resolve. label Apr 5, 2022

timhoffm commented Apr 5, 2022

Copy link
Copy Markdown
Member

@not-so-rabh could you comment on @anntzer's question?

jklymak marked this pull request as draft June 2, 2022 14:24

jklymak commented Jun 2, 2022

Copy link
Copy Markdown
Member

Converted to draft - needs discussion and rebase

tacaswell commented Aug 16, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

The issue that @anntzer points out is real, we need to only add the un-escaped font name if the name is one of the generic names.

There is also some confusion about what get_name and get_family on FontProperties do. get_family gives back the list of font names (which font tools call families) and get_name turns the crank on our font search code and return the family of the font actually found on the users system.

This line of code also was impacted by #23559 and the fix for #23492 will also touch this line. I think I see how to fix both #22528 and #23492 in one shot and in the interest of trying to squeeze these in for 3.6 am just going to do it.

Thank you for your work on this @not-so-rabh , I apologize for taking this issue from under you, and I hope we hear from you again!

tacaswell closed this Aug 16, 2022

Copy link
Copy Markdown
Member

Replaced by #23638

tacaswell added this to the unassigned milestone Sep 1, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend: svg status: needs clarification Issues that need more information to resolve. status: needs rebase

Projects

Development

Successfully merging this pull request may close these issues.

[Bug]: problem with font property in text elements of svg figures

7 participants


Back | FazBrowse Home | New Git URL