Figure.plot: Deprecate parameter "columns" to "incols" (remove in v0.6.0)

[michaelgrund]

Description of proposed changes

As discussed in #764, this PR replaces the columns parameter by incols for Figure.plot().

• Rename the parameter columns to incols in Figure.plot()
• Updates tests and examples to use the new parameter name
• Use the deprecate_parameter decorator for backward-compatibility
• Add a test for checking 'columns' backward compatibility

Reminders

• Run make format and make check to make sure the code follows the style guide.
• Add tests for new features or tests that would have caught the bug that you're fixing.
• Write detailed docstrings for all functions/methods.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash commands are:

• /format: automatically format and lint the code
• /test-gmt-dev: run full tests on the latest GMT development version

[maxrjones]

The alias name and description for incols should go in COMMON_OPTIONS in decorators.py. I will submit a separate PR for adding the incols common option to decorators.py so that it does not get complicated coordinating the timing of this PR and #1040.

[michaelgrund]

The alias name and description for incols should go in COMMON_OPTIONS in decorators.py. I will submit a separate PR for adding the incols common option to decorators.py so that it does not get complicated coordinating the timing of this PR and #1040.

Sounds great!

[michaelgrund]

Any idea why the newly added test fails here @seisman ?

[seisman]

Any idea why the newly added test fails here @seisman ?

I think I misunderstood the order Python executes decorators. I thought the execution order is bottom-to-top, but the actual execution order is top-to-bottom (see https://gist.github.com/kirsle/bf24622fc5f255256c6e for detailed explanations).

If the top-to-bottom execution order is correct, the deprecate_parameter decorator should always be put before the use_alias decorator.

Edit: After trying the examples in https://gist.github.com/kirsle/bf24622fc5f255256c6e and https://stackoverflow.com/questions/27342149/, I'm more confused about the order. The former example supports top-to-bottom order, but the latter example supports bottom-to-top order. Am I still misunderstanding it?

[weiji14]

Python decorators are applied top-down according to https://realpython.com/primer-on-python-decorators/#nesting-decorators. The implementation of @deprecate_parameter is correct-ish, but we need to move the @deprecate_parameter decorator to be after @fmt_docstring and before @use_alias.

[weiji14]

The problem is that test_plot_deprecate_columns_to_incols uses a list instead of str for the columns parameter. The correct order should be:

1. Rename 'columns' to 'incols' via @deprecate_parameter
2. Convert long alias 'incols' to GMT short alias 'i' via @use_alias
3. Let the list type argument [0, 1] be converted to str type 0,1 via @kwargs_to_strings
4. Pass in -i0,1 to lib.call_module("plot", ...)

So you'll need to move these 2 @deprecate_parameter lines to between L17 and L18.

[michaelgrund]

Thanks for your replies and the clarification @seisman and @weiji14.

[michaelgrund]

The alias name and description for incols should go in COMMON_OPTIONS in decorators.py. I will submit a separate PR for adding the incols common option to decorators.py so that it does not get complicated coordinating the timing of this PR and #1040.

Now the description for incols from COMMON_OPTIONS is used.