-
Notifications
You must be signed in to change notification settings - Fork 221
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
Breaking change in how text strings are treated #3457
Comments
Hm. You are using both single and double quotation marks ( I can confirm that the second quotation marks are now included in the text string. But I do not think that this was comunicated as a breacking change; wondering if it is related to PR #3132. For me, import pygmt
size = 1
title = "my title"
fig = pygmt.Figure()
fig.basemap(region=[-size, size] * 2, projection="X5c/1c", frame=["WSne+tthis is {:}".format(title)])
fig.show()
# OR
fig = pygmt.Figure()
fig.basemap(region=[-size, size] * 2, projection="X5c/1c", frame=[f"WSne+tthis is {title}"])
fig.show() |
Yes, I originally did this because of whitespace, but also because adding a "+" in the title string might confuse the parser. |
Also, is ignoring the quotes backword compatible with previous versions of pygmt? |
Yes, I can confirm that this breaking change was introduced in PR #3132 and we didn't realize this breaking change until now. As originally shown in issue #247, there are four possible cases when we want to specify an argument string with whitespaces. Below is a summary table for the support of different cases in different PyGMT versions. In this table:
Here are the facts that we should know:
Since single- and double-quotations are the same in Python, it makes sense to expect that cases 1 and 2 work in the same way and cases 3 and 4 also work in the same way. Prior to PyGMT v0.6.0, only case 4 works, as reported in #247 and other similar issues. Then we introduced some hacky workarounds in #1487 (first appeared in v0.6.0) and some subsequent PRs to make cases 1/2/4 work. In PR #3132 (first appeared in v0.12.0), we realized that all the hacky workarounds in #1487 are not needed as long as we change the way we pass the CLI arguments to GMT API. The new way (passing an argument list) is also more robust than the old way (passing an argument string), but as this issue has reported, it breaks case 4. As said above, cases 3 and 4 should work in the same way in Python, so I think this breaking change is breaking things in a good way.
Yes,
Yes, as shown in the above table, case 1/2 work since PyGMT v0.6.0. |
Thanks. I just modified my code to pass the title (and other arguments) without quotes and with white space. As this approach works back to 0.6, we can probably just close this. Nevertheless, I wonder if this should be mentioned somewhere in the docs. I think that most people would be hesitant to use a title with whitespace and without quotes. The real solution is probably to move away from the idiosyncratic gmt strings, but that is another issue! |
Yes, I think we should add an entry to the v0.12.0 changelog and also mention it in the upcoming v0.14.0 release. |
Description of the problem
I have a code that makes use of pygmt that works fine. After an upgrade to 0.12/0.13 however, many of my titles and axis labels have quotes around them. This is because I am specifying gmt parameters such as the title like
'+t"This is my title"'
. This used to work in all previous versions.It is not clear if this is a bug, or if this is a breaking change. If pygmt changed how they parse strings for gmt options, that is fine, but I would like to make sure before changing my code.
Minimal Complete Verifiable Example
Full error message
No response
System information
The text was updated successfully, but these errors were encountered: