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

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

Merged
merged 7 commits into from
May 28, 2021
Merged
9 changes: 5 additions & 4 deletions pygmt/src/plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
Y="yshift",
Z="zvalue",
a="aspatial",
i="columns",
i="incols",
l="label",
c="panel",
f="coltypes",
Expand All @@ -45,6 +45,7 @@
)
@kwargs_to_strings(R="sequence", c="sequence_comma", i="sequence_comma", p="sequence")
@deprecate_parameter("sizes", "size", "v0.4.0", remove_version="v0.6.0")
@deprecate_parameter("columns", "incols", "v0.4.0", remove_version="v0.6.0")
Copy link
Member

@weiji14 weiji14 May 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

def plot(self, x=None, y=None, data=None, size=None, direction=None, **kwargs):
r"""
Plot lines, polygons, and symbols in 2-D.
Expand Down Expand Up @@ -192,10 +193,10 @@ def plot(self, x=None, y=None, data=None, size=None, direction=None, **kwargs):
{a}
{c}
{f}
columns : str or 1d array
incols : str or 1d array
Choose which columns are x, y, color, and size, respectively if
input is provided via *data*. E.g. ``columns = [0, 1]`` or
``columns = '0,1'`` if the *x* values are stored in the first
input is provided via *data*. E.g. ``incols = [0, 1]`` or
``incols = '0,1'`` if the *x* values are stored in the first
column and *y* values in the second one. Note: zero-based
indexing is used.
label : str
Expand Down
33 changes: 28 additions & 5 deletions pygmt/tests/test_plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,8 +309,8 @@ def test_plot_matrix(data):
projection="M15c",
style="cc",
color="#aaaaaa",
B="a",
columns="0,1,2+s0.005",
frame="a",
incols="0,1,2+s0.005",
)
return fig

Expand All @@ -327,7 +327,7 @@ def test_plot_matrix_color(data):
projection="X10c",
style="c0.5c",
cmap="rainbow",
B="a",
frame="a",
)
return fig

Expand All @@ -345,7 +345,7 @@ def test_plot_from_file(region):
style="d1c",
color="yellow",
frame=True,
columns=[0, 1],
incols=[0, 1],
)
return fig

Expand Down Expand Up @@ -453,7 +453,7 @@ def test_plot_datetime():
@pytest.mark.mpl_image_compare(filename="test_plot_sizes.png")
def test_plot_deprecate_sizes_to_size(data, region):
"""
Make sure that the old parameter "sizes" is supported and it reports an
Make sure that the old parameter "sizes" is supported and it reports a
warning.

Modified from the test_plot_sizes() test.
Expand All @@ -472,3 +472,26 @@ def test_plot_deprecate_sizes_to_size(data, region):
)
assert len(record) == 1 # check that only one warning was raised
return fig


@pytest.mark.mpl_image_compare(filename="test_plot_from_file.png")
def test_plot_deprecate_columns_to_incols(region):
"""
Make sure that the old parameter "columns" is supported and it reports a
warning.

Modified from the test_plot_from_file() test.
"""
fig = Figure()
with pytest.warns(expected_warning=FutureWarning) as record:
fig.plot(
data=POINTS_DATA,
region=region,
projection="X10c",
style="d1c",
color="yellow",
frame=True,
columns=[0, 1],
)
assert len(record) == 1 # check that only one warning was raised
return fig