-
Notifications
You must be signed in to change notification settings - Fork 224
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
Update COMMON_OPTIONS arguments to GMT standard #783
Update COMMON_OPTIONS arguments to GMT standard #783
Conversation
I can't figure out why this is failing so many tests, as it is changing the content of strings and no functions. It appears to be rendering correctly when it is built in Sphinx. |
One doctest in
|
I misinterpreted the |
They are "known" failures.
You need to update these lines: pygmt/pygmt/helpers/decorators.py Lines 172 to 180 in 547a6fc
|
Co-authored-by: Wei Ji <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reckon this PR is about done, just a few more changes needed.
pygmt/helpers/decorators.py
Outdated
@@ -15,13 +15,14 @@ | |||
COMMON_OPTIONS = { | |||
"R": """\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"R": """\ | |
"R": r"""\ |
Need to make this a raw r-string to avoid the pylint warning (W605 invalid escape sequence '\ ')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It rendered incorrectly when I would have raw strings in both COMMON_OPTIONS and the function doc strings (check out the deployment for 62e8aad at the top). My guess is some issue with a raw string with escape characters being passed into another raw string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, might need to rethink this a bit then or just ignore the pylint
error if it's harmless.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I add a pylint: disable=invalid-escape-sequence
(is that the correct wording/syntax?)?
Co-authored-by: Wei Ji <[email protected]>
Specify the region of interest. This is a required argument if this | ||
is the first plot command.""", | ||
"J": r"""projection : str | ||
*projection*\ [*projection-specific arguments*\ ]\ *figure size*. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pygmt/helpers/decorators.py
Outdated
"XY": r"""xshift : str | ||
[**a**\|\ **c**\|\ **f**\|\ **r**\][*xshift*]. | ||
Shift plot origin in x-direction. | ||
|
||
yshift : str | ||
[**a**\|\ **c**\|\ **f**\|\ **r**\][*yshift*]. | ||
Shift plot origin in y-direction. Full documentation is at | ||
:gmt-docs:`gmt.html#xy-full`.""", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Co-authored-by: Dongdong Tian <[email protected]>
…common-options-doc-update
Update the argument doc strings in the COMMON_OPTIONS dictionary in decorators.py to the standard set in #631.