From df88e02539538e25c1c4fab2dbe2782e360d7c9c Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Mon, 3 Mar 2025 14:39:31 +0800 Subject: [PATCH 01/11] Figure.plot/plot3d/text: Pass a dict of vectors rather than x/y/extra_arrays --- pygmt/src/plot.py | 25 +++++++++++++------------ pygmt/src/plot3d.py | 31 ++++++++++++++----------------- pygmt/src/text.py | 17 ++++++++--------- 3 files changed, 35 insertions(+), 38 deletions(-) diff --git a/pygmt/src/plot.py b/pygmt/src/plot.py index 23c5bde12fd..8114f5dfec6 100644 --- a/pygmt/src/plot.py +++ b/pygmt/src/plot.py @@ -50,7 +50,7 @@ w="wrap", ) @kwargs_to_strings(R="sequence", c="sequence_comma", i="sequence_comma", p="sequence") -def plot( +def plot( # noqa: PLR0912 self, data=None, x=None, @@ -232,8 +232,8 @@ def plot( kwargs = self._preprocess(**kwargs) kind = data_kind(data) - extra_arrays = [] - if kind == "empty": # Add more columns for vectors input + if kind == "empty": # Data is given via a series of vectors. + data = {"x": x, "y": y} # Parameters for vector styles if ( isinstance(kwargs.get("S"), str) @@ -241,25 +241,28 @@ def plot( and kwargs["S"][0] in "vV" and is_nonstr_iter(direction) ): - extra_arrays.extend(direction) + data.update({"x2": direction[0], "y2": direction[1]}) # Fill if is_nonstr_iter(kwargs.get("G")): - extra_arrays.append(kwargs.get("G")) + data["fill"] = kwargs["G"] del kwargs["G"] # Size if is_nonstr_iter(size): - extra_arrays.append(size) + data["size"] = size # Intensity and transparency - for flag in ["I", "t"]: + for flag, name in ["I", "intensity"], ["t", "transparency"]: if is_nonstr_iter(kwargs.get(flag)): - extra_arrays.append(kwargs.get(flag)) + data[name] = kwargs[flag] kwargs[flag] = "" # Symbol must be at the last column if is_nonstr_iter(symbol): if "S" not in kwargs: kwargs["S"] = True - extra_arrays.append(symbol) + data["symbol"] = symbol else: + if any(v is not None for v in (x, y)): + msg = "Too much data. Use either data or x/y/z." + raise GMTInvalidInput(msg) for name, value in [ ("direction", direction), ("fill", kwargs.get("G")), @@ -277,7 +280,5 @@ def plot( kwargs["S"] = "s0.2c" with Session() as lib: - with lib.virtualfile_in( - check_kind="vector", data=data, x=x, y=y, extra_arrays=extra_arrays - ) as vintbl: + with lib.virtualfile_in(check_kind="vector", data=data) as vintbl: lib.call_module(module="plot", args=build_arg_list(kwargs, infile=vintbl)) diff --git a/pygmt/src/plot3d.py b/pygmt/src/plot3d.py index e8e75382d74..6f6a3919d76 100644 --- a/pygmt/src/plot3d.py +++ b/pygmt/src/plot3d.py @@ -51,7 +51,7 @@ w="wrap", ) @kwargs_to_strings(R="sequence", c="sequence_comma", i="sequence_comma", p="sequence") -def plot3d( +def plot3d( # noqa: PLR0912 self, data=None, x=None, @@ -210,9 +210,8 @@ def plot3d( kwargs = self._preprocess(**kwargs) kind = data_kind(data) - extra_arrays = [] - - if kind == "empty": # Add more columns for vectors input + if kind == "empty": # Data is given via a series of vectors. + data = {"x": x, "y": y, "z": z} # Parameters for vector styles if ( isinstance(kwargs.get("S"), str) @@ -220,25 +219,29 @@ def plot3d( and kwargs["S"][0] in "vV" and is_nonstr_iter(direction) ): - extra_arrays.extend(direction) + data.update({"x2": direction[0], "y2": direction[1]}) # Fill if is_nonstr_iter(kwargs.get("G")): - extra_arrays.append(kwargs.get("G")) + data["fill"] = kwargs["G"] del kwargs["G"] # Size if is_nonstr_iter(size): - extra_arrays.append(size) + data["size"] = size # Intensity and transparency - for flag in ["I", "t"]: + for flag, name in [("I", "intensity"), ("t", "transparency")]: if is_nonstr_iter(kwargs.get(flag)): - extra_arrays.append(kwargs.get(flag)) + data[name] = kwargs[flag] kwargs[flag] = "" # Symbol must be at the last column if is_nonstr_iter(symbol): if "S" not in kwargs: kwargs["S"] = True - extra_arrays.append(symbol) + data["symbol"] = symbol else: + if any(v is not None for v in (x, y, z)): + msg = "Too much data. Use either data or x/y/z." + raise GMTInvalidInput(msg) + for name, value in [ ("direction", direction), ("fill", kwargs.get("G")), @@ -257,12 +260,6 @@ def plot3d( with Session() as lib: with lib.virtualfile_in( - check_kind="vector", - data=data, - x=x, - y=y, - z=z, - extra_arrays=extra_arrays, - required_z=True, + check_kind="vector", data=data, required_z=True ) as vintbl: lib.call_module(module="plot3d", args=build_arg_list(kwargs, infile=vintbl)) diff --git a/pygmt/src/text.py b/pygmt/src/text.py index b507510f620..6eda035f025 100644 --- a/pygmt/src/text.py +++ b/pygmt/src/text.py @@ -222,22 +222,24 @@ def text_( # noqa: PLR0912 elif isinstance(arg, int | float | str): kwargs["F"] += f"{flag}{arg}" - extra_arrays = [] confdict = {} + data = None if kind == "empty": + data = {"x": x, "y": y} + for arg, flag, name in array_args: if is_nonstr_iter(arg): kwargs["F"] += flag # angle is numeric type and font/justify are str type. if name == "angle": - extra_arrays.append(arg) + data["angle"] = arg else: - extra_arrays.append(np.asarray(arg, dtype=np.str_)) + data[name] = np.asarray(arg, dtype=np.str_) # If an array of transparency is given, GMT will read it from the last numerical # column per data record. if is_nonstr_iter(kwargs.get("t")): - extra_arrays.append(kwargs["t"]) + data["transparency"] = kwargs["t"] kwargs["t"] = True # Append text to the last column. Text must be passed in as str type. @@ -247,7 +249,7 @@ def text_( # noqa: PLR0912 text, encoding=encoding ) confdict["PS_CHAR_ENCODING"] = encoding - extra_arrays.append(text) + data["text"] = text else: if isinstance(position, str): kwargs["F"] += f"+c{position}+t{text}" @@ -260,10 +262,7 @@ def text_( # noqa: PLR0912 with Session() as lib: with lib.virtualfile_in( check_kind="vector", - data=textfiles, - x=x, - y=y, - extra_arrays=extra_arrays, + data=textfiles or data, required_data=required_data, ) as vintbl: lib.call_module( From ce57c59680365e900c3244db8ef80db61008fe64 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Mon, 3 Mar 2025 15:00:35 +0800 Subject: [PATCH 02/11] Check if a dict of vectors contain None --- pygmt/helpers/utils.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/pygmt/helpers/utils.py b/pygmt/helpers/utils.py index 94c1d8a8901..09201408e20 100644 --- a/pygmt/helpers/utils.py +++ b/pygmt/helpers/utils.py @@ -12,6 +12,7 @@ import time import webbrowser from collections.abc import Iterable, Mapping, Sequence +from itertools import islice from pathlib import Path from typing import Any, Literal @@ -41,7 +42,7 @@ ] -def _validate_data_input( +def _validate_data_input( # noqa: PLR0912 data=None, x=None, y=None, z=None, required_z=False, required_data=True, kind=None ) -> None: """ @@ -143,6 +144,15 @@ def _validate_data_input( raise GMTInvalidInput(msg) if hasattr(data, "data_vars") and len(data.data_vars) < 3: # xr.Dataset raise GMTInvalidInput(msg) + if kind == "vectors" and isinstance(data, dict): + # Iterator over the up-to-3 first elements. + arrays = list(islice(data.values(), 3)) + if len(arrays) < 2 or any(v is None for v in arrays[:2]): # Check x/y + msg = "Must provide x and y." + raise GMTInvalidInput(msg) + if required_z and (len(arrays) < 3 or arrays[2] is None): # Check z + msg = "Must provide x, y, and z." + raise GMTInvalidInput(msg) def _is_printable_ascii(argstr: str) -> bool: From 2cb9295538f7b6e369ab617fe2bd67e190932e32 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Mon, 3 Mar 2025 15:01:08 +0800 Subject: [PATCH 03/11] clib.virtualfile_in: Remove the 'extra_arrays' parameter --- pygmt/clib/session.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pygmt/clib/session.py b/pygmt/clib/session.py index 9ab3e37574e..6b29d6183dd 100644 --- a/pygmt/clib/session.py +++ b/pygmt/clib/session.py @@ -1772,7 +1772,6 @@ def virtualfile_in( x=None, y=None, z=None, - extra_arrays=None, required_z=False, required_data=True, ): @@ -1794,9 +1793,6 @@ def virtualfile_in( data input. x/y/z : 1-D arrays or None x, y, and z columns as numpy arrays. - extra_arrays : list of 1-D arrays - Optional. A list of numpy arrays in addition to x, y, and z. - All of these arrays must be of the same size as the x/y/z arrays. required_z : bool State whether the 'z' column is required. required_data : bool @@ -1879,8 +1875,6 @@ def virtualfile_in( _data = [x, y] if z is not None: _data.append(z) - if extra_arrays: - _data.extend(extra_arrays) case "vectors": if hasattr(data, "items") and not hasattr(data, "to_frame"): # pandas.DataFrame or xarray.Dataset types. From 362e5bd9efa03cb30d4cf150783d4c982eade67f Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Mon, 3 Mar 2025 16:41:21 +0800 Subject: [PATCH 04/11] Figure.plot3d: Add one more test to increase code coverage --- pygmt/tests/test_plot3d.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/pygmt/tests/test_plot3d.py b/pygmt/tests/test_plot3d.py index f3a616d75e6..879b3783b9a 100644 --- a/pygmt/tests/test_plot3d.py +++ b/pygmt/tests/test_plot3d.py @@ -88,6 +88,21 @@ def test_plot3d_fail_1d_array_with_data(data, region): fig.plot3d(style="cc", fill="red", transparency=data[:, 2] * 100, **kwargs) +def test_plot3d_fail_no_data(data, region): + """ + Should raise an exception if data is not enough. + """ + fig = Figure() + with pytest.raises(GMTInvalidInput): + fig.plot3d( + style="c0.2c", x=data[0], y=data[1], region=region, projection="X10c" + ) + with pytest.raises(GMTInvalidInput): + fig.plot3d( + style="c0.2c", data=data, x=data[0], region=region, projection="X10c" + ) + + @pytest.mark.mpl_image_compare def test_plot3d_projection(data, region): """ From 8f72e4cad8a4f61fa71b4ffeaa10bbf406036e24 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Mon, 3 Mar 2025 16:55:14 +0800 Subject: [PATCH 05/11] Clarify that dictionary will be recognized as vectors --- pygmt/clib/session.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pygmt/clib/session.py b/pygmt/clib/session.py index 6b29d6183dd..8153d4dbbce 100644 --- a/pygmt/clib/session.py +++ b/pygmt/clib/session.py @@ -1877,7 +1877,7 @@ def virtualfile_in( _data.append(z) case "vectors": if hasattr(data, "items") and not hasattr(data, "to_frame"): - # pandas.DataFrame or xarray.Dataset types. + # Dictionary, pandas.DataFrame or xarray.Dataset types. # pandas.Series will be handled below like a 1-D numpy.ndarray. _data = [array for _, array in data.items()] else: From c199dbf88712277eaf8b5937de9bd5c29156bb29 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Thu, 6 Mar 2025 10:09:08 +0800 Subject: [PATCH 06/11] Clarify that the data parameter can accept dicts --- pygmt/clib/session.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pygmt/clib/session.py b/pygmt/clib/session.py index 8153d4dbbce..4556c0d9ce3 100644 --- a/pygmt/clib/session.py +++ b/pygmt/clib/session.py @@ -1787,7 +1787,7 @@ def virtualfile_in( check_kind : str or None Used to validate the type of data that can be passed in. Choose from 'raster', 'vector', or None. Default is None (no validation). - data : str or pathlib.Path or xarray.DataArray or {table-like} or None + data : str or pathlib.Path or xarray.DataArray or {table-like} or dict | None Any raster or vector data format. This could be a file name or path, a raster grid, a vector matrix/arrays, or other supported data input. From 1d0d0dcff12b90e260ac6d3f86d5d29d2791dded Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Mon, 10 Mar 2025 15:52:16 +0800 Subject: [PATCH 07/11] Fix a typo --- pygmt/clib/session.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pygmt/clib/session.py b/pygmt/clib/session.py index 4556c0d9ce3..e8f6299885b 100644 --- a/pygmt/clib/session.py +++ b/pygmt/clib/session.py @@ -1787,7 +1787,7 @@ def virtualfile_in( check_kind : str or None Used to validate the type of data that can be passed in. Choose from 'raster', 'vector', or None. Default is None (no validation). - data : str or pathlib.Path or xarray.DataArray or {table-like} or dict | None + data : str or pathlib.Path or xarray.DataArray or {table-like} or dict or None Any raster or vector data format. This could be a file name or path, a raster grid, a vector matrix/arrays, or other supported data input. From e44e712115359a0e2211ff810c4d975679f97dfb Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Wed, 26 Mar 2025 16:02:18 +0800 Subject: [PATCH 08/11] Improve docstrings for the new test --- pygmt/tests/test_plot3d.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pygmt/tests/test_plot3d.py b/pygmt/tests/test_plot3d.py index 879b3783b9a..a1f4e306d41 100644 --- a/pygmt/tests/test_plot3d.py +++ b/pygmt/tests/test_plot3d.py @@ -90,7 +90,7 @@ def test_plot3d_fail_1d_array_with_data(data, region): def test_plot3d_fail_no_data(data, region): """ - Should raise an exception if data is not enough. + Should raise an exception if data is not enough or too much. """ fig = Figure() with pytest.raises(GMTInvalidInput): From 5330d0ac4c388bd107877582fefafc353219b4c1 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Wed, 26 Mar 2025 16:32:24 +0800 Subject: [PATCH 09/11] Raise warnings when extra_arrays is used --- pygmt/clib/session.py | 16 ++++++++++++++ pygmt/tests/test_clib_virtualfile_in.py | 28 +++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/pygmt/clib/session.py b/pygmt/clib/session.py index e8f6299885b..03f3843ee75 100644 --- a/pygmt/clib/session.py +++ b/pygmt/clib/session.py @@ -1774,6 +1774,7 @@ def virtualfile_in( z=None, required_z=False, required_data=True, + extra_arrays=None, ): """ Store any data inside a virtual file. @@ -1798,6 +1799,13 @@ def virtualfile_in( required_data : bool Set to True when 'data' is required, or False when dealing with optional virtual files. [Default is True]. + extra_arrays : list of 1-D arrays + Optional. A list of numpy arrays in addition to x, y, and z. All of these + arrays must be of the same size as the x/y/z arrays. + + .. deprecated:: v0.16.0 + The parameter 'extra_arrays' will be removed in v0.20.0. Prepare and pass + a dictionary of arrays instead. E.g., `{"x": x, "y": y, "size": size}`. Returns ------- @@ -1875,6 +1883,14 @@ def virtualfile_in( _data = [x, y] if z is not None: _data.append(z) + if extra_arrays: + msg = ( + "The parameter 'extra_arrays' will be removed in v0.20.0. " + "Prepare and pass a dictionary of arrays instead. E.g., " + "`{'x': x, 'y': y, 'size': size}`." + ) + warnings.warn(message=msg, category=FutureWarning, stacklevel=1) + _data.extend(extra_arrays) case "vectors": if hasattr(data, "items") and not hasattr(data, "to_frame"): # Dictionary, pandas.DataFrame or xarray.Dataset types. diff --git a/pygmt/tests/test_clib_virtualfile_in.py b/pygmt/tests/test_clib_virtualfile_in.py index 8a43c1dc273..dd885f56f0f 100644 --- a/pygmt/tests/test_clib_virtualfile_in.py +++ b/pygmt/tests/test_clib_virtualfile_in.py @@ -128,3 +128,31 @@ def test_virtualfile_in_matrix_string_dtype(): assert output == "347.5 348.5 -30.5 -30\n" # Should check that lib.virtualfile_from_vectors is called once, # not lib.virtualfile_from_matrix, but it's technically complicated. + + +def test_virtualfile_in_extra_arrays(data): + """ + Test that the extra_arrays parameter is deprecated. + """ + with clib.Session() as lib: + # Call the method twice to ensure only one statement in the with block. + # Test that a FutureWarning is raised when extra_arrays is used. + with pytest.warns(FutureWarning): + with lib.virtualfile_in( + check_kind="vector", + x=data[:, 0], + y=data[:, 1], + extra_arrays=[data[:, 2]], + ) as vfile: + pass + # Test that the output is correct. + with GMTTempFile() as outfile: + with lib.virtualfile_in( + check_kind="vector", + x=data[:, 0], + y=data[:, 1], + extra_arrays=[data[:, 2]], + ) as vfile: + lib.call_module("info", [vfile, "-C", f"->{outfile.name}"]) + output = outfile.read(keep_tabs=False) + assert output == "11.5309 61.7074 -2.9289 7.8648 0.1412 0.9338\n" From 0a63ef0a34ffccd920e25aa0ce747e9f602e5200 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Wed, 26 Mar 2025 16:43:23 +0800 Subject: [PATCH 10/11] Add TODO comments --- pygmt/clib/session.py | 1 + pygmt/tests/test_clib_virtualfile_in.py | 1 + 2 files changed, 2 insertions(+) diff --git a/pygmt/clib/session.py b/pygmt/clib/session.py index 03f3843ee75..572e318f895 100644 --- a/pygmt/clib/session.py +++ b/pygmt/clib/session.py @@ -1883,6 +1883,7 @@ def virtualfile_in( _data = [x, y] if z is not None: _data.append(z) + # TODO(PyGMT>=0.20.0): Remove the deprecated parameter 'extra_arrays'. if extra_arrays: msg = ( "The parameter 'extra_arrays' will be removed in v0.20.0. " diff --git a/pygmt/tests/test_clib_virtualfile_in.py b/pygmt/tests/test_clib_virtualfile_in.py index dd885f56f0f..bf7c54b1bba 100644 --- a/pygmt/tests/test_clib_virtualfile_in.py +++ b/pygmt/tests/test_clib_virtualfile_in.py @@ -130,6 +130,7 @@ def test_virtualfile_in_matrix_string_dtype(): # not lib.virtualfile_from_matrix, but it's technically complicated. +# TODO(PyGMT>=0.20.0): Remove the test related to deprecated parameter 'extra_arrays'. def test_virtualfile_in_extra_arrays(data): """ Test that the extra_arrays parameter is deprecated. From 9163be5b494fa0fe34d551bb2a93abdb93240c99 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Fri, 28 Mar 2025 21:35:00 +0800 Subject: [PATCH 11/11] Move TODO comment to the top --- pygmt/clib/session.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pygmt/clib/session.py b/pygmt/clib/session.py index 572e318f895..330591de81a 100644 --- a/pygmt/clib/session.py +++ b/pygmt/clib/session.py @@ -1765,6 +1765,7 @@ def virtualfile_from_stringio( seg.header = None seg.text = None + # TODO(PyGMT>=0.20.0): Remove the deprecated parameter 'extra_arrays'. def virtualfile_in( self, check_kind=None, @@ -1800,12 +1801,12 @@ def virtualfile_in( Set to True when 'data' is required, or False when dealing with optional virtual files. [Default is True]. extra_arrays : list of 1-D arrays - Optional. A list of numpy arrays in addition to x, y, and z. All of these - arrays must be of the same size as the x/y/z arrays. + A list of numpy arrays in addition to x, y, and z. All of these arrays must + be of the same size as the x/y/z arrays. .. deprecated:: v0.16.0 The parameter 'extra_arrays' will be removed in v0.20.0. Prepare and pass - a dictionary of arrays instead. E.g., `{"x": x, "y": y, "size": size}`. + a dictionary of arrays instead. E.g., ``{"x": x, "y": y, "size": size}``. Returns ------- @@ -1883,7 +1884,6 @@ def virtualfile_in( _data = [x, y] if z is not None: _data.append(z) - # TODO(PyGMT>=0.20.0): Remove the deprecated parameter 'extra_arrays'. if extra_arrays: msg = ( "The parameter 'extra_arrays' will be removed in v0.20.0. "