GenericMappingTools / pygmt

A Python interface for the Generic Mapping Tools.
https://www.pygmt.org
BSD 3-Clause "New" or "Revised" License
747 stars 216 forks source link

Decorator kwargs_to_strings is too aggressive when converting boolean arguments #640

Closed seisman closed 3 years ago

seisman commented 3 years ago

Description of the problem

https://github.com/GenericMappingTools/pygmt/blob/c191d3e09a43a0536b2ae14a7c8380969f4646a4/pygmt/helpers/decorators.py#L279

Decorator kwargs_to_strings can deal with boolean arguments. For example, dict {'A': True, 'B': False, 'R': '0/10/0/10'} will be converted to {'A': '', 'R': '0/10/0/10'} first, and then build_arg_strings will further convert it to valid GMT arguments -A -R0/10/0/10.

The decorator works well for arguments that will be passed to GMT, but doesn't work for pure Python arguments.

Full code that generated the error

from pprint import pprint
from pygmt.helpers.decorators import kwargs_to_strings

@kwargs_to_strings(R='sequence', files='sequence_space')
def module(nongmt_args=True, *args, **kwargs):
    "A module that prints the arguments it received"
    pprint(args)
    pprint(kwargs)
    if nongmt_args is True:
        print("nongmt_args is still True. Do something in PyGMT.")
    elif nongmt_args == "":
        print("the kwargs_to_strings decorator converts nongmt_args to an empty string.")
    else:
        print(f"nongmt_args is '{nongmt_args}'")

module(R=[1, 2, 3, 4], nongmt_args=True)

Output

()
{'R': '1/2/3/4'}
the kwargs_to_strings decorator converts nongmt_args to an empty string.

As shown in the output above, the boolean argument nongmt_args is converted from True to an empty string "". Thus, checking nongmt_args is True no longer works.

If removing the kwargs_to_strings decorator before the module function, the output is:

()
{'R': [1, 2, 3, 4]}
nongmt_args is still True. Do something in PyGMT.

Possible solution

Like what we do in #639, the possible solution is to convert boolean arguments in build_arg_strings function before passing arguments to GMT, not in the kwargs_to_strings decorator.

System information

Please paste the output of python -c "import pygmt; pygmt.show_versions()":

PyGMT information:
  version: v0.2.0+19.gc191d3e0
System information:
  python: 3.8.3 (default, Jul  2 2020, 11:26:31)  [Clang 10.0.0 ]
  executable: /Users/seisman/.anaconda/bin/python
  machine: macOS-10.15.6-x86_64-i386-64bit
Dependency information:
  numpy: 1.18.5
  pandas: 1.0.5
  xarray: 0.16.1
  netCDF4: 1.5.3
  packaging: 20.4
  ghostscript: 9.53.1
  gmt: 6.2.0_612863b_2020.10.03
GMT library information:
  binary dir: /Users/seisman/.anaconda/bin
  cores: 8
  grid layout: rows
  library path: /Users/seisman/local/GMT/lib/libgmt.dylib
  padding: 2
  plugin dir: /Users/seisman/local/GMT/lib/gmt/plugins
  share dir: /Users/seisman/local/GMT/share
  version: 6.2.0
seisman commented 3 years ago

Ping @leouieda as he wrote the original codes of the decorator.