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

Session.virtualfile_in: Deprecate the parameter 'extra_arrays. Prepare and pass a dictionary of arrays instead (will be removed in v0.20.0) #3823

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions pygmt/clib/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -1772,9 +1772,9 @@ def virtualfile_in(
x=None,
y=None,
z=None,
extra_arrays=None,
required_z=False,
required_data=True,
Comment on lines 1772 to 1776
Copy link
Member

@weiji14 weiji14 Mar 5, 2025

Choose a reason for hiding this comment

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

should raise warnings when extra_arrays is used and keep backward compatibility for a few versions (maybe 4 versions).

If we're going to break compatibility of virtualfile_in for 4 minor versions, maybe it's a good time to rethink the parameter ordering (ties in with potential changes in #3369). Would it make sense for the signature to be something like:

def virtualfile_in(
    self,
    check_kind=None,
    required_data=True,
    required_z=False,
    data=None,
    x=None,
    y=None,
    z=None,
    **extra_arrays

? We could also mark some of those parameters as keyword only (https://peps.python.org/pep-0570/#keyword-only-arguments) to ensure future compatibility (i.e. prevent positional-only parameters/arguments in case we decide to change the order again).

Copy link
Member Author

Choose a reason for hiding this comment

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

Please see #3836 for my thoughts on the Session.virtualfile_in method.

extra_arrays=None,
):
"""
Store any data inside a virtual file.
Expand All @@ -1788,20 +1788,24 @@ 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 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.
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
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
-------
Expand Down Expand Up @@ -1879,11 +1883,18 @@ 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. "
"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"):
# 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:
Expand Down
12 changes: 11 additions & 1 deletion pygmt/helpers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
"""
Expand Down Expand Up @@ -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):
Copy link
Member Author

Choose a reason for hiding this comment

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

Please note that, these new codes in _validate_data_input is temporary and I plan to refactor _validate_data_input in a separate PR.

# 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:
Expand Down
25 changes: 13 additions & 12 deletions pygmt/src/plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -232,34 +232,37 @@ 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)
and len(kwargs["S"]) >= 1
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")),
Expand All @@ -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))
31 changes: 14 additions & 17 deletions pygmt/src/plot3d.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -210,35 +210,38 @@ 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)
and len(kwargs["S"]) >= 1
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")),
Expand All @@ -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))
17 changes: 8 additions & 9 deletions pygmt/src/text.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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}"
Expand All @@ -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(
Expand Down
29 changes: 29 additions & 0 deletions pygmt/tests/test_clib_virtualfile_in.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,3 +128,32 @@ 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.


# 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.
"""
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"
15 changes: 15 additions & 0 deletions pygmt/tests/test_plot3d.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 or too much.
"""
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):
"""
Expand Down