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

Fail for invalid input arguments #256

Open
seisman opened this issue Dec 7, 2018 · 3 comments
Open

Fail for invalid input arguments #256

seisman opened this issue Dec 7, 2018 · 3 comments
Labels
bug Something isn't working help wanted Helping hands are appreciated longterm Long standing issues that need to be resolved
Milestone

Comments

@seisman
Copy link
Member

seisman commented Dec 7, 2018

Description of the problem

We should be more careful to pass unrecognized keyword parameters to gmt command.

Full code that generated the error

import gmt

# Load sample earthquake data in a pandas.DataFrame
quakes = gmt.datasets.load_usgs_quakes()

# Load the builtin Earth relief grid as an xarray.DataArray.
relief = gmt.datasets.load_earth_relief(resolution="30m")

# The Figure object controls all plotting functions
fig = gmt.Figure()
# Setup a map with a global region, a Mollweide projection, and automatic ticks
fig.basemap(region="g", projection="W200/8i", frame=True)
# Plot the Earth relief grid in pseudo-color.
fig.grdimage(relief, cmap="geo")
# Plot earthquakes as circles. Size maps to magnitude and color to depth.
fig.plot(x=quakes.longitude, y=quakes.latitude, sizes=0.01*2**quakes.mag,
         colors=quakes.depth/quakes.depth.max(), cmap="viridis", style="cc")
# Show a preview of the image (inline if in a Jupyter notebook).
fig.show()

The code above is the same as the code on gmt-python's homepage, except that in fig.plot(), I use the keyword colors instead of color. I know it's a user's mistake, but gmt-python reports some strange errors, which is very confusing.

Full error message

gmt-python-session [ERROR]: Current panel has row or column outsiden range!
psxy [ERROR]: Found no history for option -c
[Session gmt-python-session (9)]: Error returned from GMT API: GMT_OPTION_HISTORY_ERROR (63)
---------------------------------------------------------------------------
GMTCLibError                              Traceback (most recent call last)
<ipython-input-1-5a482bf0dd97> in <module>()
     15 # Plot earthquakes as circles. Size maps to magnitude and color to depth.
     16 fig.plot(x=quakes.longitude, y=quakes.latitude, sizes=0.01*2**quakes.mag,
---> 17          colors=quakes.depth/quakes.depth.max(), cmap="viridis", style="cc")
     18 # Show a preview of the image (inline if in a Jupyter notebook).
     19 fig.show()

~/Gits/gmt/gmt-python/gmt/helpers/decorators.py in new_module(*args, **kwargs)
    197                 if alias in kwargs:
    198                     kwargs[arg] = kwargs.pop(alias)
--> 199             return module_func(*args, **kwargs)
    200
    201         new_module.aliases = aliases

~/Gits/gmt/gmt-python/gmt/helpers/decorators.py in new_module(*args, **kwargs)
    292                         )
    293             # Execute the original function and return its output
--> 294             return module_func(*args, **kwargs)
    295
    296         return new_module

~/Gits/gmt/gmt-python/gmt/base_plotting.py in plot(self, x, y, data, sizes, direction, **kwargs)
    344             with file_context as fname:
    345                 arg_str = " ".join([fname, build_arg_string(kwargs)])
--> 346                 lib.call_module("plot", arg_str)
    347
    348     @fmt_docstring

~/Gits/gmt/gmt-python/gmt/clib/session.py in call_module(self, module, args)
    488             raise GMTCLibError(
    489                 "Module '{}' failed with status code {}:\n{}".format(
--> 490                     module, status, self._error_message
    491                 )
    492             )

GMTCLibError: Module 'plot' failed with status code 63:
gmt-python-session [ERROR]: Current panel has row or column outsiden range!
psxy [ERROR]: Found no history for option -c

fig.plot() cannot recognize the keyword colors, then pass it directly to gmt plot. Thus the gmt cmd looks like gmt plot ... -colors, which is a wrong syntax for gmt plot.

For unrecognized keyword parameters, maybe we can pass keyword parameters only if the keyword has a single character, and report other unrecognized parameters to users.

System information

  • Operating system: macOS
  • Python installation (Anaconda, system, ETS): Anaconda
  • Version of GMT: 6.0.0 latest
  • Version of Python: 3.7.0
  • Version of this package: latest
  • If using conda, paste the output of conda list below:
@leouieda
Copy link
Member

This is a great example of why I'm very unhappy with our current way of handling arguments. If we were using normal Python arguments, this typo would have been easily caught by the Python interpreter. Instead, we would have to implement our own argument checking for every single function because of our decorators for input parsing. I'm playing around with ideas for changing this. Mainly having normal keyword Python arguments and translating things inside the Python function. See #262

@leouieda leouieda added the bug Something isn't working label Jan 22, 2019
@leouieda leouieda changed the title Be careful with passing unrecognized keyword parameters to gmt command Fail for invalid input arguments Jan 22, 2019
@weiji14 weiji14 added help wanted Helping hands are appreciated longterm Long standing issues that need to be resolved labels Feb 17, 2021
This was referenced Feb 23, 2021
@weiji14 weiji14 added this to the 1.0.0 milestone Nov 3, 2021
@seisman
Copy link
Member Author

seisman commented Mar 1, 2022

I still have the same issue now and then due to typos. I don't think we can work out a better way handling parameters in the next few releases, but at least we can have a workaround:

For unrecognized keyword parameters, maybe we can pass keyword parameters only if the keyword has a single character, and report other unrecognized parameters to users.

I still think this is the easiest workaround. What we should do is add a check at the start of the build_arg_string function, like:

   for key, value in kwargs.items():
       if len(key) > 2: 
           raise GMTInvalidInput(f"Unrecognized parameter {key}.")

here I use len(key) > 2 rather than len(key) > 1, because I know sometimes user use Tm="..." when calling coast.

@seisman
Copy link
Member Author

seisman commented Mar 8, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Helping hands are appreciated longterm Long standing issues that need to be resolved
Projects
None yet
Development

No branches or pull requests

3 participants