Skip to content

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

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

Merged
merged 25 commits into from
Apr 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
df88e02
Figure.plot/plot3d/text: Pass a dict of vectors rather than x/y/extra…
seisman Mar 3, 2025
ce57c59
Check if a dict of vectors contain None
seisman Mar 3, 2025
2cb9295
clib.virtualfile_in: Remove the 'extra_arrays' parameter
seisman Mar 3, 2025
362e5bd
Figure.plot3d: Add one more test to increase code coverage
seisman Mar 3, 2025
8f72e4c
Clarify that dictionary will be recognized as vectors
seisman Mar 3, 2025
f017adb
Merge branch 'main' into refactor/virtualfile_in_extra_arrays
seisman Mar 6, 2025
c199dbf
Clarify that the data parameter can accept dicts
seisman Mar 6, 2025
07924ca
Merge branch 'main' into refactor/virtualfile_in_extra_arrays
seisman Mar 10, 2025
1d0d0dc
Fix a typo
seisman Mar 10, 2025
a7ee253
Merge branch 'main' into refactor/virtualfile_in_extra_arrays
seisman Mar 24, 2025
5cd8275
Merge branch 'main' into refactor/virtualfile_in_extra_arrays
seisman Mar 26, 2025
e44e712
Improve docstrings for the new test
seisman Mar 26, 2025
5330d0a
Raise warnings when extra_arrays is used
seisman Mar 26, 2025
0a63ef0
Add TODO comments
seisman Mar 26, 2025
9163be5
Move TODO comment to the top
seisman Mar 28, 2025
7a48006
Merge branch 'main' into refactor/virtualfile_in_extra_arrays
seisman Mar 28, 2025
7153d4f
Merge branch 'main' into refactor/virtualfile_in_extra_arrays
seisman Mar 31, 2025
36327da
Merge branch 'main' into refactor/virtualfile_in_extra_arrays
seisman Mar 31, 2025
c3aae9c
Merge branch 'main' into refactor/virtualfile_in_extra_arrays
seisman Apr 2, 2025
065c490
Merge branch 'main' into refactor/virtualfile_in_extra_arrays
seisman Apr 3, 2025
dd9ec47
Merge branch 'main' into refactor/virtualfile_in_extra_arrays
seisman Apr 4, 2025
7d08dd2
Merge branch 'main' into refactor/virtualfile_in_extra_arrays
seisman Apr 7, 2025
0d9007f
Merge branch 'main' into refactor/virtualfile_in_extra_arrays
seisman Apr 15, 2025
195b746
Apply suggestions from code review
seisman Apr 15, 2025
177d56a
Apply suggestions from code review
seisman Apr 15, 2025
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
24 changes: 18 additions & 6 deletions pygmt/clib/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -1748,16 +1748,17 @@ 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,
data=None,
x=None,
y=None,
z=None,
extra_arrays=None,
required_z=False,
required_data=True,
Comment on lines 1756 to 1760
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 @@ -1771,20 +1772,25 @@ 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
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 to the `data` parameter. E.g.,
``data={"x": x, "y": y, "size": size}``.

Returns
-------
Expand Down Expand Up @@ -1863,10 +1869,16 @@ def virtualfile_in(
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 to the `data` "
"parameter. E.g., `data={'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
26 changes: 13 additions & 13 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,36 @@ 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"))
del kwargs["G"]
data["fill"] = kwargs.pop("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 +279,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))
32 changes: 14 additions & 18 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,37 @@ 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"))
del kwargs["G"]
data["fill"] = kwargs.pop("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 +259,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
Loading