GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Doc: tell 3.11 upgraders to remove the pre-shaping workaround by Syamjith-NK · Pull Request #32263 · matplotlib/matplotlib Conversation
|
|
Thank you for opening your first PR into Matplotlib! If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks. We also ask that you please finish addressing any review comments on this PR and wait for it to be merged (or closed) before opening a new one, as it can be a valuable learning experience to go through the review process. You can also join us on discourse chat 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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Answering the AI question in the template, since it was asked directly. The finding is mine. I hit it building something in Arabic that looked wrong to me, then measured it against a Pillow/Raqm render of the same string in the same font, and wrote the reproducer in #32262, which runs on my own machine. I used an AI assistant to help draft and tighten the wording of this section, and to check that the module docstring still parsed as RST before I pushed. The technical claims I checked myself, including reading Happy to rewrite any of the prose if it does not sound like the rest of the page. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
To verify there is no generic way to determine if an Arabic, Hebrew, or Devanagari string is pre-ordered from logical to display order? How big of a problem/common is it that applications have pre-reversed strings kicking around? Per https://www.w3.org/International/questions/qa-bidi-unicode-controls.en.html#basedirection if the user wraps pre-reversed text in the LRO/PDF unicode points the text will render correctly in all versions of mpl (because <3.11 we ignore the bidirectional details all together and >= 3.11 we (via libraqm) respect them correctly). import matplotlib import matplotlib.pyplot as plt # editors / GH will correctly render RTL so use \N to be clear # hello = "שלום" hello = "\N{HEBREW LETTER SHIN}\N{HEBREW LETTER LAMED}\N{HEBREW LETTER VAV}\N{HEBREW LETTER FINAL MEM}" hello_r = hello[::-1] hello_fixed = "\N{LRO}" + hello_r + "\N{PDF}" fig, ax = plt.subplots() ax.set_title(f'{matplotlib.__version__=}') ax.text(0, .7, "mpl direct") ax.text(.5, .7, hello, fontsize=30) ax.text(0, .5, "pre-flipped") ax.text(.5, .5, hello_r, fontsize=30) ax.text(0, .25, "pre-flipped + LRO/PDF") ax.text(.5, .25, hello_fixed, fontsize=30) plt.show() fig.savefig(f'/tmp/bidi_mpl{matplotlib.__version__.replace('.', '-')}.png')
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks, all three of these are more useful than what I filed. On LRO/PDF with Arabic. You said you were not confident, so I checked it. It works, with one caveat I think is worth writing down. I rendered
The reading is correct in all nine, including with a lam-alef ligature and with Latin digits mixed in, so your recommendation holds. But it is not always the same rendering. The wrapped string draws the font's static Presentation Forms glyphs instead of the font's own shaping, so wherever those are not metrically identical to the shaped output you get looser joins and a wider line. With Geeza Pro that is visible at a glance, not just in the numbers. So for the third branch I would document it as "renders correctly on every version" rather than "renders identically". For someone who does not control the matplotlib calls that still seems like clearly the right trade. Is there a generic way to tell a string is pre-ordered? Partly, and the split is the interesting part.
I wrapped the Forms-B check in a small linter and ran it across 3,883 mixed files here: 31 flagged spans, all true positives. How common is it? A floor, counting only public code that GitHub indexes: 1,172 files match @rcomer thank you for editing those StackOverflow answers. That reaches more people than this PR will, and I should have thought to do it myself. @story645 that makes sense, and it is a better home than where I put it. I will rewrite this as an admonition on the 3.11 change note covering @tacaswell's three branches, and drop the section I added to the fonts page unless you would still like something on RTL there. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Moves the note into an admonition on the 3.11 change note, per review, and covers the three cases: pin to 3.11, version-gate, or wrap the pre-processed string in LRO/PDF when the call cannot be changed.
| Arabic, Persian, Urdu and Hebrew was to reorder the string before passing it in, | ||
| typically with ``arabic_reshaper`` and ``python-bidi``:: | ||
|
|
||
| ax.set_title(get_display(arabic_reshaper.reshape(text))) |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
| ax.set_title(get_display(arabic_reshaper.reshape(text))) | |
| preprocessed = get_display(arabic_reshaper.reshape(text)) | |
| ax.set_title(preprocessed) |
so it's clear you're using the same thing below with the direction overrides.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Applied, thanks — that was a real gap and not just style: the override snippet further down already referred to preprocessed, so with the expression inlined above it the name was never bound and the two examples did not read as the same value.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Please don't reply by copy-pasting an LLM; it just repeated what I said with more words, and isn't even right about what it's saying.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Apply QuLogic's review suggestion. The later LEFT-TO-RIGHT OVERRIDE example already referred to `preprocessed`, but the line above inlined the expression and never bound that name, so the two snippets did not read as the same value.
| Arabic, Persian, Urdu and Hebrew was to reorder the string before passing it in, | ||
| typically with ``arabic_reshaper`` and ``python-bidi``:: | ||
|
|
||
| preprocessed = get_display(arabic_reshaper.reshape(text)) |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
do we want to include the imports? On the hand, it makes the code more understandable to people who are not already using these, but on the other hand if you do not already recognize these functions, this note is not for you.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I think it is likely fine without the imports, though get_display is a little generic of a name.
Overall, though, the context is pretty clear and this is an example of workarounds that people previously did, not a suggestion of something to do, which makes it easier to justify a shorter snippet.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
…263-on-v3.11.x Backport PR #32263 on branch v3.11.x (Doc: tell 3.11 upgraders to remove the pre-shaping workaround)
…263-on-v3.11.0-doc Backport PR #32263 on branch v3.11.0-doc (Doc: tell 3.11 upgraders to remove the pre-shaping workaround)
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
[Doc]: 3.11 text shaping silently reverses the arabic_reshaper + python-bidi workaround
There was an error while loading. Please reload this page.