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

Decorator kwargs_to_strings is too aggressive when converting boolean arguments #640

Closed
seisman opened this issue Oct 4, 2020 · 1 comment · Fixed by #1125
Closed

Decorator kwargs_to_strings is too aggressive when converting boolean arguments #640

seisman opened this issue Oct 4, 2020 · 1 comment · Fixed by #1125
Assignees
Labels
bug Something isn't working
Milestone

Comments

@seisman
Copy link
Member

seisman commented Oct 4, 2020

Description of the problem

def kwargs_to_strings(convert_bools=True, **conversions):

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
@weiji14 weiji14 added the bug Something isn't working label Oct 5, 2020
@seisman
Copy link
Member Author

seisman commented Oct 5, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants