From 2dd366ddc50a547bc70066980211814714fddbd4 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Thu, 25 Mar 2021 01:33:40 -0400 Subject: [PATCH 01/10] Refactor text to use virtual_from_data --- pygmt/src/text.py | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/pygmt/src/text.py b/pygmt/src/text.py index 4c7b5421410..690082f7471 100644 --- a/pygmt/src/text.py +++ b/pygmt/src/text.py @@ -7,7 +7,6 @@ from pygmt.helpers import ( build_arg_string, data_kind, - dummy_context, fmt_docstring, is_nonstr_iter, kwargs_to_strings, @@ -157,15 +156,17 @@ def text_( # Ensure inputs are either textfiles, x/y/text, or position/text if position is None: kind = data_kind(textfiles, x, y, text) + if kind == "vectors" and text is None: + raise GMTInvalidInput("Must provide text with x/y pairs") else: - if x is not None or y is not None: + if x is not None or y is not None or textfiles is not None: raise GMTInvalidInput( - "Provide either position only, or x/y pairs, not both" + "Provide either position only, or x/y pairs, or textfiles." ) - kind = "vectors" - - if kind == "vectors" and text is None: - raise GMTInvalidInput("Must provide text with x/y pairs or position") + if text is None or is_nonstr_iter(text): + raise GMTInvalidInput("Text can't be None or array.") + kind = "file" + textfiles = "" # Build the -F option in gmt text. if "F" not in kwargs.keys() and ( @@ -193,19 +194,15 @@ def text_( extra_arrays.append(kwargs["t"]) kwargs["t"] = "" + # Append text as the last column + # text must be in str type, see issue #706 + if kind == "vectors": + extra_arrays.append(np.atleast_1d(text).astype(str)) + with Session() as lib: - file_context = dummy_context(textfiles) if kind == "file" else "" - if kind == "vectors": - if position is not None: - file_context = dummy_context("") - else: - file_context = lib.virtualfile_from_vectors( - np.atleast_1d(x), - np.atleast_1d(y), - *extra_arrays, - # text must be in str type, see issue #706 - np.atleast_1d(text).astype(str), - ) + file_context = lib.virtualfile_from_data( + check_kind="vector", data=textfiles, x=x, y=y, extra_arrays=extra_arrays + ) with file_context as fname: arg_str = " ".join([fname, build_arg_string(kwargs)]) lib.call_module("text", arg_str) From 206c5cc418e3a94ab32ca69e4eebdc39778c53cc Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Thu, 25 Mar 2021 02:16:16 -0400 Subject: [PATCH 02/10] Add tests for invalid inputs --- pygmt/tests/test_text.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pygmt/tests/test_text.py b/pygmt/tests/test_text.py index 8c3305c5267..c8d8ff52d56 100644 --- a/pygmt/tests/test_text.py +++ b/pygmt/tests/test_text.py @@ -134,6 +134,14 @@ def test_text_xy_with_position_fails(region): fig.text( region=region, projection="x1c", x=1.2, y=2.4, position="MC", text="text" ) + with pytest.raises(GMTInvalidInput): + fig.text(region=region, projection="x1c", textfiles="file.txt", text="text") + with pytest.raises(GMTInvalidInput): + fig.text(region=region, projection="x1c", position="MC", text=None) + with pytest.raises(GMTInvalidInput): + fig.text( + region=region, projection="x1c", position="MC", text=["text1", "text2"] + ) @pytest.mark.mpl_image_compare From 8763889abdd51bcdcbf1cfd7a93c1190c528dd49 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Fri, 26 Mar 2021 00:14:46 -0400 Subject: [PATCH 03/10] Fix test name --- pygmt/tests/test_text.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pygmt/tests/test_text.py b/pygmt/tests/test_text.py index c8d8ff52d56..e7e84cdb7da 100644 --- a/pygmt/tests/test_text.py +++ b/pygmt/tests/test_text.py @@ -125,9 +125,9 @@ def test_text_position(region): return fig -def test_text_xy_with_position_fails(region): +def test_text_invalid_inputs(region): """ - Run text by providing both x/y pairs and position arguments. + Run text by providing invalid combinations of inputs. """ fig = Figure() with pytest.raises(GMTInvalidInput): From 4b3d92af6474905e1b5d3cbf69baf27b602bfa5f Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Sat, 14 May 2022 18:10:23 +0800 Subject: [PATCH 04/10] Apply suggestions from code review Co-authored-by: Max Jones --- pygmt/src/text.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pygmt/src/text.py b/pygmt/src/text.py index b0902e3c406..4b7e87c91ec 100644 --- a/pygmt/src/text.py +++ b/pygmt/src/text.py @@ -171,6 +171,8 @@ def text_( # Ensure inputs are either textfiles, x/y/text, or position/text if position is None: + if (x is not None or y is not None) and textfiles is not None: + raise GMTInvalidInput( "Provide either position only, or x/y pairs, or textfiles.") kind = data_kind(textfiles, x, y, text) if kind == "vectors" and text is None: raise GMTInvalidInput("Must provide text with x/y pairs") @@ -181,7 +183,7 @@ def text_( ) if text is None or is_nonstr_iter(text): raise GMTInvalidInput("Text can't be None or array.") - kind = "file" + kind = "" textfiles = "" # Build the -F option in gmt text. @@ -220,8 +222,7 @@ def text_( extra_arrays.append(kwargs["t"]) kwargs["t"] = "" - # Append text as the last column - # text must be in str type, see issue #706 + # Append text as the last column. Text must be passed in str type. if kind == "vectors": extra_arrays.append(np.atleast_1d(text).astype(str)) From cb6bf1e7e1f1f740fc009d01a443bba98ca7cc4f Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Sat, 14 May 2022 18:15:23 +0800 Subject: [PATCH 05/10] Fix formatting issues --- pygmt/src/text.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pygmt/src/text.py b/pygmt/src/text.py index 4b7e87c91ec..4bad03f9122 100644 --- a/pygmt/src/text.py +++ b/pygmt/src/text.py @@ -172,7 +172,9 @@ def text_( # Ensure inputs are either textfiles, x/y/text, or position/text if position is None: if (x is not None or y is not None) and textfiles is not None: - raise GMTInvalidInput( "Provide either position only, or x/y pairs, or textfiles.") + raise GMTInvalidInput( + "Provide either position only, or x/y pairs, or textfiles." + ) kind = data_kind(textfiles, x, y, text) if kind == "vectors" and text is None: raise GMTInvalidInput("Must provide text with x/y pairs") From acebf962c17b385ac034cc8cc11b0da4390fe7a2 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Sat, 14 May 2022 18:32:17 +0800 Subject: [PATCH 06/10] Add one more test to increase coverage --- pygmt/tests/test_text.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pygmt/tests/test_text.py b/pygmt/tests/test_text.py index 718c379eb1a..b82948c6992 100644 --- a/pygmt/tests/test_text.py +++ b/pygmt/tests/test_text.py @@ -141,6 +141,8 @@ def test_text_invalid_inputs(region): fig.text( region=region, projection="x1c", position="MC", text=["text1", "text2"] ) + with pytest.raises(GMTInvalidInput): + fig.text(region=region, projection="x1c", textfiles="file.txt", x=1.2, y=2.4) @pytest.mark.mpl_image_compare From a800de52e3a208f285608bc4a13990594ebfd2fd Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Sat, 14 May 2022 18:41:42 +0800 Subject: [PATCH 07/10] Refactor test fixture to fix the linting issue --- pygmt/tests/test_text.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pygmt/tests/test_text.py b/pygmt/tests/test_text.py index b82948c6992..c2ad692f5b5 100644 --- a/pygmt/tests/test_text.py +++ b/pygmt/tests/test_text.py @@ -1,4 +1,3 @@ -# pylint: disable=redefined-outer-name """ Tests text. """ @@ -15,16 +14,16 @@ CITIES_DATA = os.path.join(TEST_DATA_DIR, "cities.txt") -@pytest.fixture(scope="module") -def projection(): +@pytest.fixture(scope="module", name="projection") +def fixture_projection(): """ The projection system. """ return "x10c" -@pytest.fixture(scope="module") -def region(): +@pytest.fixture(scope="module", name="region") +def fixture_region(): """ The data region. """ From b927fd9c27582749deb95822868beb7f006a1c63 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Sat, 14 May 2022 18:42:39 +0800 Subject: [PATCH 08/10] Remove a few blank lines --- pygmt/tests/test_text.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pygmt/tests/test_text.py b/pygmt/tests/test_text.py index c2ad692f5b5..f725c768f2d 100644 --- a/pygmt/tests/test_text.py +++ b/pygmt/tests/test_text.py @@ -322,10 +322,8 @@ def test_text_transparency(): text = [f"TEXT-{i}-{j}" for i, j in zip(x, y)] fig = Figure() - fig.basemap(region=[0, 10, 10, 20], projection="X10c", frame=True) fig.text(x=x, y=y, text=text, transparency=50) - return fig @@ -342,7 +340,6 @@ def test_text_varying_transparency(): fig = Figure() fig.basemap(region=[0, 10, 10, 20], projection="X10c", frame=True) fig.text(x=x, y=y, text=text, transparency=transparency) - return fig @@ -366,7 +363,6 @@ def test_text_nonstr_text(): Input text is in non-string type (e.g., int, float) """ fig = Figure() - fig.text( region=[0, 10, 0, 10], projection="X10c", @@ -375,5 +371,4 @@ def test_text_nonstr_text(): y=[1, 2, 3, 4], text=[1, 2, 3.0, 4.0], ) - return fig From ff4712515a1733821c668fb0fed0f590567cf3e0 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Sun, 15 May 2022 14:45:26 +0800 Subject: [PATCH 09/10] Remove an unnecessary pylint flag --- pygmt/src/text.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/pygmt/src/text.py b/pygmt/src/text.py index 4bad03f9122..e92d9c153c8 100644 --- a/pygmt/src/text.py +++ b/pygmt/src/text.py @@ -163,10 +163,7 @@ def text_( for texts, but this option is only valid if using x/y/text. {w} """ - - # pylint: disable=too-many-locals # pylint: disable=too-many-branches - kwargs = self._preprocess(**kwargs) # pylint: disable=protected-access # Ensure inputs are either textfiles, x/y/text, or position/text From 5768d5055fc2a31a0661b9ecf3f9005f0baa98b9 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Mon, 16 May 2022 09:31:15 +0800 Subject: [PATCH 10/10] Apply suggestions from code review Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com> --- pygmt/src/text.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pygmt/src/text.py b/pygmt/src/text.py index e92d9c153c8..2cde7480d19 100644 --- a/pygmt/src/text.py +++ b/pygmt/src/text.py @@ -182,7 +182,7 @@ def text_( ) if text is None or is_nonstr_iter(text): raise GMTInvalidInput("Text can't be None or array.") - kind = "" + kind = None textfiles = "" # Build the -F option in gmt text. @@ -221,7 +221,7 @@ def text_( extra_arrays.append(kwargs["t"]) kwargs["t"] = "" - # Append text as the last column. Text must be passed in str type. + # Append text at last column. Text must be passed in as str type. if kind == "vectors": extra_arrays.append(np.atleast_1d(text).astype(str))