Ensure non-ASCII characters are typeset correctly even if PS_CHAR_ENCODING is not 'ISOLatin1+' #3611
Conversation
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
seisman
force-pushed
the
enhancement/char-encoding
branch
from
1ad9d70 to
a575805
Compare
seisman
force-pushed
the
enhancement/char-encoding
branch
from
a575805 to
8c34683
Compare
…ODING is not 'ISOLatin1+'
seisman
added
enhancement
seisman
commented
Nov 12, 2024
| Default PS_CHAR_ENCODING setting should not affect the result. | ||
| """ | ||
| fig = Figure() | ||
| if encoding == "Standard+": # Temporarily set the PS_CHAR_ENCODING to "Standard+". |
Member
Author
There was a problem hiding this comment.
Ideally, we should test the case that PS_CHAR_ENCODING is set to "Standard+" in a global "gmt.conf" file. However, it's technically difficult. Similar to the test test_gmt_compat_6_is_applied at
Generally we need to:
- Generate a gmt.conf file in the current directory
- Kill the global session
- Start a new session
- Create a new figure and adding non-ASCII characters to it
- End the session so the "gmt.conf" file won't affect other tests
- Start a new, clean global session
But the issue is that, after starting a new global session, we can't return the Figure instance for comparing images, because mpl_image_compare needs to call Figure.savefig which no longer works after the session is destroied.
So here we only test the case that PS_CHAR_ENCODING is changed in the middle of a script.
michaelgrund
approved these changes
Nov 14, 2024
seisman
added
final review call
Member
Author
|
I'll merge this PR in 24 hours. |
seisman
removed
the
final review call
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of proposed changes
Non-ASCII characters may not be typeset correctly if the default PS_CHAR_ENCODING setting is not
ISOLatin1+. This can happen when (1) GMT is compiled with US units as default; (2) users have a globalgmt.conffile withPS_CHAR_ENCODINGset toStandard+; (3) users setPS_CHAR_ENCODINGtoStandard+in the middle of a script.Taking the gallery example at https://www.pygmt.org/dev/tutorials/advanced/non_ascii_text.html as an example.
If you have a
gmt.conffile withPS_CHAR_ENCODINGset toStandard+(e.g., you can create agmt.conffile by running GMT commandgmt set PS_CHAR_ENCODING=Standard+in terminal), running the gallery example will result in the incorrect figure shown in the left column below:Previously, we assumed the default PS_CHAR_ENCODING is
ISOLatin1+and didn't add--PS_CHAR_ENCODING=ISOLatin1+if the text string contains non-ASCII characters in the ISOLatin1+ charset. This PR fixes the issue by always adding--PS_CHAR_ENCODING=<encoding>even if encoding isISOLatin1+.In 8c34683, the existing test
test_text_nonasciiis modified to show that settingPS_CHAR_ENCODINGtoStandard+breaks the test (https://github.com/GenericMappingTools/pygmt/actions/runs/11800822308/job/32872646796?pr=3611). The fix is added in d9e5203.Closes #2204.