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

Raise GMTInvalidInput exception when required z is missing #1478

Merged
merged 27 commits into from
Sep 15, 2021
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
dc11bb6
add required_z parameters, and if statement to handle missing z vector
yohaimagen Sep 1, 2021
df1b0d5
add required_z parameters and pass it to data_kind
yohaimagen Sep 1, 2021
1b7a8a6
test requierd_z
yohaimagen Sep 2, 2021
e471f9e
Update pygmt/helpers/utils.py
yohaimagen Sep 3, 2021
11d98ee
Update pygmt/helpers/utils.py
yohaimagen Sep 3, 2021
bcfc03a
Update pygmt/helpers/utils.py
yohaimagen Sep 3, 2021
ba73d5b
checks that when required z data matrix has at least 3 columns
yohaimagen Sep 3, 2021
ed1419d
[format-command] fixes
actions-bot Sep 3, 2021
166b439
add required_z=True to _blockm
yohaimagen Sep 3, 2021
7675449
add required_z=True to contour
yohaimagen Sep 3, 2021
09c68cd
add required_z=True to plot3d
yohaimagen Sep 3, 2021
8b3eeb2
add required_z=True to surface
yohaimagen Sep 3, 2021
2a77656
reformat
yohaimagen Sep 3, 2021
e1d8248
remove unnecessary import
yohaimagen Sep 3, 2021
15a5bf2
Merge branch 'main' into raise_GMTInvalidInput_exception_when_z_missing
yohaimagen Sep 4, 2021
8aa06cf
Merge branch 'main' into raise_GMTInvalidInput_exception_when_z_missing
yohaimagen Sep 12, 2021
306a5c6
add required_z to wiggle
yohaimagen Sep 12, 2021
8451465
remove insuficent data test from test_contour
yohaimagen Sep 12, 2021
da51487
add test for sufficent data to test_clib
yohaimagen Sep 12, 2021
4559284
remove test for sufficent data test_plot3d
yohaimagen Sep 12, 2021
6a0042e
remove test for passing z value test_surface
yohaimagen Sep 12, 2021
bb51fc5
reformat
yohaimagen Sep 12, 2021
5eb4cc0
remove unused imports
yohaimagen Sep 12, 2021
ba10dbb
Apply suggestions from code review
yohaimagen Sep 14, 2021
1639a5b
Merge branch 'main' into raise_GMTInvalidInput_exception_when_z_missing
yohaimagen Sep 14, 2021
15075e8
Apply suggestions from code review
yohaimagen Sep 14, 2021
91c446f
Merge branch 'main' into raise_GMTInvalidInput_exception_when_z_missing
seisman Sep 15, 2021
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
11 changes: 9 additions & 2 deletions pygmt/clib/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -1362,7 +1362,14 @@ def virtualfile_from_grid(self, grid):

@fmt_docstring
def virtualfile_from_data(
self, check_kind=None, data=None, x=None, y=None, z=None, extra_arrays=None
self,
check_kind=None,
data=None,
x=None,
y=None,
z=None,
extra_arrays=None,
required_z=False,
):
"""
Store any data inside a virtual file.
Expand Down Expand Up @@ -1415,7 +1422,7 @@ def virtualfile_from_data(
...
<vector memory>: N = 3 <7/9> <4/6> <1/3>
"""
kind = data_kind(data, x, y, z)
kind = data_kind(data, x, y, z, required_z=required_z)

if check_kind == "raster" and kind not in ("file", "grid"):
raise GMTInvalidInput(f"Unrecognized data type for grid: {type(data)}")
Expand Down
8 changes: 6 additions & 2 deletions pygmt/helpers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from pygmt.exceptions import GMTInvalidInput


def data_kind(data, x=None, y=None, z=None):
def data_kind(data, x=None, y=None, z=None, required_z=False):
"""
Check what kind of data is provided to a module.

Expand Down Expand Up @@ -69,7 +69,9 @@ def data_kind(data, x=None, y=None, z=None):
if data is not None and (x is not None or y is not None or z is not None):
raise GMTInvalidInput("Too much data. Use either data or x and y.")
if data is None and (x is None or y is None):
raise GMTInvalidInput("Must provided both x and y.")
raise GMTInvalidInput("Must provide both x and y.")
if data is None and required_z and z is None:
raise GMTInvalidInput("Must provide x, y, and z.")

if isinstance(data, (str, pathlib.PurePath)):
kind = "file"
Expand All @@ -78,6 +80,8 @@ def data_kind(data, x=None, y=None, z=None):
elif hasattr(data, "__geo_interface__"):
kind = "geojson"
elif data is not None:
if required_z and data.shape[1] < 3:
Copy link
Member

@weiji14 weiji14 Sep 18, 2021

Choose a reason for hiding this comment

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

Checking for data.shape[1] fails for xarray.Dataset inputs with AttributeError: 'Dataset' object has no attribute 'shape'. The shape attribute is valid only for numpy.ndarray, pandas.DataFrame, and maybe a few other PyData objects. Will need to create a bugfix and add some extra tests for xarray.Dataset inputs.

Copy link
Member

@maxrjones maxrjones Sep 18, 2021

Choose a reason for hiding this comment

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

I can submit a PR fix since I think that I suggested this. But I feel that we should not label it as a bug since this code has not yet been included in a release, so it might be confusing to list on the changelog as a bugfix relative to v0.4.1.

Edit: Just noticed the comment on the other PR that @weiji14 would fix this. That's also fine.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I've submitted a bugfix PR at #1523. Have put on the skip-changelog label so it won't show up in the changelog :)

raise GMTInvalidInput("data must provide x, y, and z columns.")
kind = "matrix"
else:
kind = "vectors"
Expand Down
2 changes: 1 addition & 1 deletion pygmt/src/blockm.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def _blockm(block_method, table, outfile, x, y, z, **kwargs):
with Session() as lib:
# Choose how data will be passed into the module
table_context = lib.virtualfile_from_data(
check_kind="vector", data=table, x=x, y=y, z=z
check_kind="vector", data=table, x=x, y=y, z=z, required_z=True
)
# Run blockm* on data table
with table_context as infile:
Expand Down
5 changes: 1 addition & 4 deletions pygmt/src/contour.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
"""

from pygmt.clib import Session
from pygmt.exceptions import GMTInvalidInput
from pygmt.helpers import (
build_arg_string,
data_kind,
Expand Down Expand Up @@ -127,9 +126,7 @@ def contour(self, x=None, y=None, z=None, data=None, **kwargs):
"""
kwargs = self._preprocess(**kwargs) # pylint: disable=protected-access

kind = data_kind(data, x, y, z)
if kind == "vectors" and z is None:
raise GMTInvalidInput("Must provided both x, y, and z.")
kind = data_kind(data, x, y, z, required_z=True)

with Session() as lib:
# Choose how data will be passed in to the module
Expand Down
8 changes: 7 additions & 1 deletion pygmt/src/plot3d.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,13 @@ def plot3d(
with Session() as lib:
# Choose how data will be passed in to the module
file_context = lib.virtualfile_from_data(
check_kind="vector", data=data, x=x, y=y, z=z, extra_arrays=extra_arrays
check_kind="vector",
data=data,
x=x,
y=y,
z=z,
extra_arrays=extra_arrays,
required_z=True,
)

with file_context as fname:
Expand Down
4 changes: 1 addition & 3 deletions pygmt/src/surface.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,7 @@ def surface(x=None, y=None, z=None, data=None, **kwargs):
- None if ``outgrid`` is set (grid output will be stored in file set by
``outgrid``)
"""
kind = data_kind(data, x, y, z)
if kind == "vectors" and z is None:
raise GMTInvalidInput("Must provide z with x and y.")
kind = data_kind(data, x, y, z, required_z=True)

with GMTTempFile(suffix=".nc") as tmpfile:
with Session() as lib:
Expand Down
2 changes: 1 addition & 1 deletion pygmt/src/wiggle.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ def wiggle(self, x=None, y=None, z=None, data=None, **kwargs):
with Session() as lib:
# Choose how data will be passed in to the module
file_context = lib.virtualfile_from_data(
check_kind="vector", data=data, x=x, y=y, z=z
check_kind="vector", data=data, x=x, y=y, z=z, required_z=True
)

with file_context as fname:
Expand Down
64 changes: 64 additions & 0 deletions pygmt/tests/test_clib.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
# pylint: disable=protected-access
# pylint: disable=redefined-outer-name
"""
Test the wrappers for the C API.
"""
import os
from contextlib import contextmanager
from itertools import product

import numpy as np
import numpy.testing as npt
Expand All @@ -23,11 +25,20 @@
from pygmt.helpers import GMTTempFile

TEST_DATA_DIR = os.path.join(os.path.dirname(__file__), "data")
POINTS_DATA = os.path.join(TEST_DATA_DIR, "points.txt")

with clib.Session() as _lib:
gmt_version = Version(_lib.info["version"])


@pytest.fixture(scope="module")
def data():
"""
Load the point data from the test file.
"""
return np.loadtxt(POINTS_DATA)


@contextmanager
def mock(session, func, returns=None, mock_func=None):
"""
Expand Down Expand Up @@ -410,6 +421,59 @@ def test_virtual_file_bad_direction():
print("This should have failed")


def test_virtualfile_from_data_required_z_matrix():
"""
Test that function fails when third z column in a matrix is needed but not
provided.
"""

yohaimagen marked this conversation as resolved.
Show resolved Hide resolved
data = np.ones((5, 2))
with clib.Session() as lib:
with pytest.raises(GMTInvalidInput):
with lib.virtualfile_from_data(data=data, required_z=True):
print("This should have failed")
yohaimagen marked this conversation as resolved.
Show resolved Hide resolved


def test_virtualfile_from_data_fail_non_valid_data(data):
"""
Should raise an exception if too few or too much data is given.
"""
# Test all combinations where at least one data variable
# is not given in the x, y case:
for variable in product([None, data[:, 0]], repeat=2):
# Filter one valid configuration:
if not any(item is None for item in variable):
continue
with clib.Session() as lib:
with pytest.raises(GMTInvalidInput):
lib.virtualfile_from_data(
x=variable[0],
y=variable[1],
)

# Test all combinations where at least one data variable
# is not given in the x, y, z case:
for variable in product([None, data[:, 0]], repeat=3):
# Filter one valid configuration:
if not any(item is None for item in variable):
continue
with clib.Session() as lib:
with pytest.raises(GMTInvalidInput):
lib.virtualfile_from_data(
x=variable[0], y=variable[1], z=variable[2], required_z=True
)

# Should also fail if given too much data
with clib.Session() as lib:
with pytest.raises(GMTInvalidInput):
lib.virtualfile_from_data(
x=data[:, 0],
y=data[:, 1],
z=data[:, 2],
data=data,
)


def test_virtualfile_from_vectors():
"""
Test the automation for transforming vectors to virtual file dataset.
Expand Down
42 changes: 0 additions & 42 deletions pygmt/tests/test_contour.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,10 @@
Tests contour.
"""
import os
from itertools import product

import numpy as np
import pytest
from pygmt import Figure
from pygmt.exceptions import GMTInvalidInput

TEST_DATA_DIR = os.path.join(os.path.dirname(__file__), "data")
POINTS_DATA = os.path.join(TEST_DATA_DIR, "points.txt")
Expand All @@ -30,46 +28,6 @@ def region():
return [10, 70, -5, 10]


def test_contour_fail_no_data(data):
"""
Should raise an exception if no data is given.
"""
# Contour should raise an exception if no or not sufficient data
# is given
fig = Figure()
# Test all combinations where at least one data variable
# is not given:
for variable in product([None, data[:, 0]], repeat=3):
# Filter one valid configuration:
if not any(item is None for item in variable):
continue
with pytest.raises(GMTInvalidInput):
fig.contour(
x=variable[0],
y=variable[1],
z=variable[2],
region=region,
projection="X4i",
color="red",
frame="afg",
pen="",
)
# Should also fail if given too much data
with pytest.raises(GMTInvalidInput):
fig.contour(
x=data[:, 0],
y=data[:, 1],
z=data[:, 2],
data=data,
region=region,
projection="X10c",
style="c0.2c",
color="red",
frame="afg",
pen=True,
)


@pytest.mark.mpl_image_compare
def test_contour_vec(region):
"""
Expand Down
42 changes: 0 additions & 42 deletions pygmt/tests/test_plot3d.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,48 +68,6 @@ def test_plot3d_red_circles_zsize(data, region):
return fig


def test_plot3d_fail_no_data(data, region):
"""
Plot should raise an exception if no data is given.
"""
fig = Figure()
with pytest.raises(GMTInvalidInput):
fig.plot3d(
region=region, projection="X10c", style="c0.2c", color="red", frame="afg"
)
with pytest.raises(GMTInvalidInput):
fig.plot3d(
x=data[:, 0],
region=region,
projection="X10c",
style="c0.2c",
color="red",
frame="afg",
)
with pytest.raises(GMTInvalidInput):
fig.plot3d(
y=data[:, 0],
region=region,
projection="X10c",
style="c0.2c",
color="red",
frame="afg",
)
# Should also fail if given too much data
with pytest.raises(GMTInvalidInput):
fig.plot3d(
x=data[:, 0],
y=data[:, 1],
z=data[:, 2],
data=data,
region=region,
projection="X10c",
style="c0.2c",
color="red",
frame="afg",
)


def test_plot3d_fail_1d_array_with_data(data, region):
"""
Should raise an exception if array color, size, intensity and transparency
Expand Down
13 changes: 0 additions & 13 deletions pygmt/tests/test_surface.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,6 @@ def test_surface_input_xyz(ship_data):
return output


def test_surface_input_xy_no_z(ship_data):
"""
Run surface by passing in x and y, but no z.
"""
with pytest.raises(GMTInvalidInput):
surface(
x=ship_data.longitude,
y=ship_data.latitude,
spacing="5m",
region=[245, 255, 20, 30],
)


def test_surface_wrong_kind_of_input(ship_data):
"""
Run surface using grid input that is not file/matrix/vectors.
Expand Down