Skip to content
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

Convert booleans arguments in build_arg_string, not in kwargs_to_strings #1125

Merged
merged 11 commits into from Apr 5, 2021
2 changes: 1 addition & 1 deletion pygmt/datasets/earth_relief.py
Expand Up @@ -10,7 +10,7 @@
from pygmt.src import grdcut, which


@kwargs_to_strings(convert_bools=False, region="sequence")
@kwargs_to_strings(region="sequence")
def load_earth_relief(resolution="01d", region=None, registration=None, use_srtm=False):
r"""
Load Earth relief grids (topography and bathymetry) in various resolutions.
Expand Down
46 changes: 8 additions & 38 deletions pygmt/helpers/decorators.py
Expand Up @@ -302,14 +302,14 @@ def new_module(*args, **kwargs):
return alias_decorator


def kwargs_to_strings(convert_bools=True, **conversions):
def kwargs_to_strings(**conversions):
"""
Decorator to convert given keyword arguments to strings.

The strings are what GMT expects from command line arguments.

Converts all boolean arguments by default. Transforms ``True`` into ``''``
(empty string) and removes the argument from ``kwargs`` if ``False``.
Boolean arguments and None are not converted and will be processed in the
``build_arg_string`` function.

You can also specify other conversions to specific arguments.

Expand All @@ -323,9 +323,6 @@ def kwargs_to_strings(convert_bools=True, **conversions):

Parameters
----------
convert_bools : bool
If ``True``, convert all boolean arguments to strings using the rules
specified above. If ``False``, leave them as they are.
conversions : keyword arguments
Keyword arguments specifying other kinds of conversions that should be
performed. The keyword is the name of the argument and the value is the
Expand Down Expand Up @@ -356,16 +353,18 @@ def kwargs_to_strings(convert_bools=True, **conversions):
>>> module(R="5/6/7/8")
{'R': '5/6/7/8'}
>>> module(P=True)
{'P': ''}
{'P': True}
>>> module(P=False)
{}
{'P': False}
>>> module(P=None)
{'P': None}
>>> module(i=[1, 2])
{'i': '1,2'}
>>> module(files=["data1.txt", "data2.txt"])
{'files': 'data1.txt data2.txt'}
>>> # Other non-boolean arguments are passed along as they are
>>> module(123, bla=(1, 2, 3), foo=True, A=False, i=(5, 6))
{'bla': (1, 2, 3), 'foo': '', 'i': '5,6'}
{'A': False, 'bla': (1, 2, 3), 'foo': True, 'i': '5,6'}
args: 123
>>> import datetime
>>> module(
Expand Down Expand Up @@ -416,8 +415,6 @@ def new_module(*args, **kwargs):
"""
New module instance that converts the arguments first.
"""
if convert_bools:
kwargs = remove_bools(kwargs)
for arg, fmt in conversions.items():
if arg in kwargs:
value = kwargs[arg]
Expand All @@ -442,30 +439,3 @@ def new_module(*args, **kwargs):
return new_module

return converter


def remove_bools(kwargs):
"""
Remove booleans from arguments.

If ``True``, replace it with an empty string. If ``False``, completely
remove the entry from the argument list.

Parameters
----------
kwargs : dict
Dictionary with the keyword arguments.

Returns
-------
new_kwargs : dict
A copy of `kwargs` with the booleans parsed.
"""
new_kwargs = {}
for arg, value in kwargs.items():
if isinstance(value, bool):
if value:
new_kwargs[arg] = ""
else:
new_kwargs[arg] = value
return new_kwargs
39 changes: 26 additions & 13 deletions pygmt/helpers/utils.py
Expand Up @@ -106,7 +106,8 @@ def build_arg_string(kwargs):
Transform keyword arguments into a GMT argument string.

Make sure all arguments have been previously converted to a string
representation using the ``kwargs_to_strings`` decorator.
representation using the ``kwargs_to_strings`` decorator. The only
exceptions are True, False and None.

Any lists or tuples left will be interpreted as multiple entries for the
same command line argument. For example, the kwargs entry ``'B': ['xa',
Expand All @@ -128,10 +129,20 @@ def build_arg_string(kwargs):

>>> print(
... build_arg_string(
... dict(R="1/2/3/4", J="X4i", P="", E=200, X=None, Y=None)
... dict(
... A=True,
... B=False,
... E=200,
... J="X4c",
... P="",
... R="1/2/3/4",
... X=None,
... Y=None,
... Z=0,
... )
... )
... )
-E200 -JX4i -P -R1/2/3/4
-A -E200 -JX4c -P -R1/2/3/4 -Z0
>>> print(
... build_arg_string(
... dict(
Expand All @@ -142,20 +153,22 @@ def build_arg_string(kwargs):
... )
... )
... )
-Bxaf -Byaf -BWSen -I1/1p,blue -I2/0.25p,blue -JX4i -R1/2/3/4
-BWSen -Bxaf -Byaf -I1/1p,blue -I2/0.25p,blue -JX4i -R1/2/3/4
"""
sorted_args = []
for key in sorted(kwargs):
gmt_args = []
# Exclude arguments that are None and False
filtered_kwargs = {
k: v for k, v in kwargs.items() if (v is not None and v is not False)
}
for key in filtered_kwargs:
if is_nonstr_iter(kwargs[key]):
for value in kwargs[key]:
sorted_args.append("-{}{}".format(key, value))
elif kwargs[key] is None: # arguments like -XNone are invalid
continue
gmt_args.append(f"-{key}{value}")
elif kwargs[key] is True:
gmt_args.append(f"-{key}")
else:
sorted_args.append("-{}{}".format(key, kwargs[key]))

arg_str = " ".join(sorted_args)
return arg_str
gmt_args.append(f"-{key}{kwargs[key]}")
return " ".join(sorted(gmt_args))


def is_nonstr_iter(value):
Expand Down
5 changes: 3 additions & 2 deletions pygmt/src/subplot.py
Expand Up @@ -148,8 +148,9 @@ def subplot(self, nrows=1, ncols=1, **kwargs):
{XY}
"""
kwargs = self._preprocess(**kwargs) # pylint: disable=protected-access
# allow for spaces in string with needing double quotes
kwargs["A"] = f'"{kwargs.get("A")}"' if kwargs.get("A") is not None else None
# allow for spaces in string without needing double quotes
if isinstance(kwargs.get("A"), str):
kwargs["A"] = f'"{kwargs.get("A")}"'
kwargs["T"] = f'"{kwargs.get("T")}"' if kwargs.get("T") else None

if nrows < 1 or ncols < 1:
Expand Down
18 changes: 14 additions & 4 deletions pygmt/src/text.py
Expand Up @@ -177,13 +177,23 @@ def text_(
)
):
kwargs.update({"F": ""})
if angle is not None and isinstance(angle, (int, float, str)):

if angle is True:
kwargs["F"] += "+a"
elif isinstance(angle, (int, float, str)):
kwargs["F"] += f"+a{str(angle)}"
Comment on lines +180 to 184
Copy link
Member

Choose a reason for hiding this comment

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

Just thinking ahead to resolve #483 (allowing list inputs to angle/list/justify), we could simply use if angle (returns True for list/int/float/str inputs) like so:

Suggested change
if angle is True:
kwargs["F"] += "+a"
elif isinstance(angle, (int, float, str)):
kwargs["F"] += f"+a{str(angle)}"
if angle:
kwargs["F"] += "+a"
if isinstance(angle, (int, float, str)):
kwargs["F"] += str(angle)

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, I was planing to work on #483, but I realized that the Figure.text() function needs more refactoring, so I opened PR #1121.

Personally, I prefer to make the changes to text.py as small as possible, and will address #483 after #1121 is merged.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, as you wish then.

if font is not None and isinstance(font, str):

if font is True:
kwargs["F"] += "+f"
elif isinstance(font, str):
kwargs["F"] += f"+f{font}"
Comment on lines +186 to 189
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if font is True:
kwargs["F"] += "+f"
elif isinstance(font, str):
kwargs["F"] += f"+f{font}"
if font:
kwargs["F"] += "+f"
if isinstance(font, str):
kwargs["F"] += font

if justify is not None and isinstance(justify, str):

if justify is True:
kwargs["F"] += "+j"
elif isinstance(justify, str):
kwargs["F"] += f"+j{justify}"
Comment on lines +191 to 194
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if justify is True:
kwargs["F"] += "+j"
elif isinstance(justify, str):
kwargs["F"] += f"+j{justify}"
if justify:
kwargs["F"] += "+j"
if isinstance(justify, str):
kwargs["F"] += justify

if position is not None and isinstance(position, str):

if isinstance(position, str):
kwargs["F"] += f'+c{position}+t"{text}"'

extra_arrays = []
Expand Down
17 changes: 0 additions & 17 deletions pygmt/tests/test_helpers.py
Expand Up @@ -50,23 +50,6 @@ def test_kwargs_to_strings_fails():
kwargs_to_strings(bla="blablabla")


def test_kwargs_to_strings_no_bools():
"""
Test that not converting bools works.
"""

@kwargs_to_strings(convert_bools=False)
def my_module(**kwargs):
"""
Function that does nothing.
"""
return kwargs

# The module should return the exact same arguments it was given
args = dict(P=True, A=False, R="1/2/3/4")
assert my_module(**args) == args


def test_gmttempfile():
"""
Check that file is really created and deleted.
Expand Down