Convert booleans arguments in build_arg_string, not in kwargs_to_stri…

…ngs (GenericMappingTools#1125)

* Do not convert booleans in the kwargs_to_strings decorator
* Deal with boolean values in build_arg_string
* load_earth_relief no longer need the convert_bools=False hack
* Fix the weird codes in subplot autolabel
* Fix the weird codes in text

Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com>

pygmt/datasets/earth_relief.py

@@ -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.

pygmt/helpers/decorators.py

@@ -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.
 
@@ -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
@@ -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(
@@ -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]
@@ -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

pygmt/helpers/utils.py

@@ -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',
@@ -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(
@@ -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):

pygmt/src/subplot.py

@@ -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:

pygmt/src/text.py

@@ -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)}"
-    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}"
-    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}"
-    if position is not None and isinstance(position, str):
+
+    if isinstance(position, str):
         kwargs["F"] += f'+c{position}+t"{text}"'
 
     extra_arrays = []

pygmt/tests/test_helpers.py

@@ -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.