From b8db7f4b6773fa8f75083fd3853a01cfb0f0cad7 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Thu, 21 Mar 2024 17:24:26 +0800 Subject: [PATCH 1/8] Session.call_module: Support passing a list of argument strings --- pygmt/clib/session.py | 53 ++++++++++++++++++++++++++++++++----------- 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/pygmt/clib/session.py b/pygmt/clib/session.py index 0640ba6ae20..edaaef7997e 100644 --- a/pygmt/clib/session.py +++ b/pygmt/clib/session.py @@ -592,22 +592,31 @@ def get_common(self, option): # the function return value (i.e., 'status') return status - def call_module(self, module, args): + def call_module(self, module: str, args: str | list[str]): """ Call a GMT module with the given arguments. - Makes a call to ``GMT_Call_Module`` from the C API using mode - ``GMT_MODULE_CMD`` (arguments passed as a single string). + Wraps ``GMT_Call_Module``. - Most interactions with the C API are done through this function. + The ``GMT_Call_Module`` API function supports passing module arguments in three + different ways: + + 1. Pass a single string that contains whitespace-separated module arguments. + 2. Pass a list of strings and each string contains a module argument. + 3. Pass a list of ``GMT_OPTION`` data structure. + + Both options 1 and 2 are implemented in this function, but option 2 is preferred + because it can correctly handle special characters like whitespaces and + quotation marks in module arguments. Parameters ---------- - module : str - Module name (``'coast'``, ``'basemap'``, etc). - args : str - String with the command line arguments that will be passed to the - module (for example, ``'-R0/5/0/10 -JM'``). + module + The GMT module name to be called (``"coast"``, ``"basemap"``, etc). + args + Module arguments that will be passed to the GMT module. It can be either + a single string (e.g., ``"-R0/5/0/10 -JX10c -BWSen+t'My Title'"``) or a list + of strings (e.g., ``["-R0/5/0/10", "-JX10c", "-BWSEN+tMy Title"]``). Raises ------ @@ -620,10 +629,28 @@ def call_module(self, module, args): restype=ctp.c_int, ) - mode = self["GMT_MODULE_CMD"] - status = c_call_module( - self.session_pointer, module.encode(), mode, args.encode() - ) + # 'args' can be (1) a single string or (2) a list of strings. + if isinstance(args, str): + # 'args' is a single string that contains whitespace-separated arguments. + # In this way, we need to correctly handle option arguments that contains + # whitepsaces or quotation marks. It's used in PyGMT <= v0.11.0 but is no + # longer recommended. + mode = self["GMT_MODULE_CMD"] + argv = args.encode() + elif isinstance(args, list): + # 'args' is a list of strings and each string contains a module argument. + # In this way, GMT can correctly handle option arguments with whitespaces or + # quotation marks. This is the preferred way to pass arguments to GMT API + # and is used for PyGMT >= v0.12.0. + mode = len(args) # 'mode' is the number of arguments. + # Pass a null pointer if no arguments are specified. + argv = strings_to_ctypes_array(args) if mode != 0 else None + else: + raise GMTInvalidInput( + "`args' must be either a string or a list of strings." + ) + + status = c_call_module(self.session_pointer, module.encode(), mode, argv) if status != 0: raise GMTCLibError( f"Module '{module}' failed with status code {status}:\n{self._error_message}" From 07ccde49ae1264b6a43915912179e2e164711ceb Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Tue, 26 Mar 2024 12:06:39 +0800 Subject: [PATCH 2/8] Add more tests for Session.call_module --- pygmt/tests/test_clib.py | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/pygmt/tests/test_clib.py b/pygmt/tests/test_clib.py index 2f655435ee4..5f1c3365357 100644 --- a/pygmt/tests/test_clib.py +++ b/pygmt/tests/test_clib.py @@ -133,9 +133,20 @@ def test_destroy_session_fails(): @pytest.mark.benchmark def test_call_module(): """ - Run a command to see if call_module works. + Call a GMT module by passing a list of arguments. + """ + with clib.Session() as lib: + with GMTTempFile() as out_fname: + lib.call_module("info", [str(POINTS_DATA), "-C", f"->{out_fname.name}"]) + assert Path(out_fname.name).stat().st_size > 0 + output = out_fname.read().strip() + assert output == "11.5309 61.7074 -2.9289 7.8648 0.1412 0.9338" + + +def test_call_module_argument_string(): + """ + Call a GMT module by passing a single argument string. """ - out_fname = "test_call_module.txt" with clib.Session() as lib: with GMTTempFile() as out_fname: lib.call_module("info", f"{POINTS_DATA} -C ->{out_fname.name}") @@ -144,9 +155,19 @@ def test_call_module(): assert output == "11.5309 61.7074 -2.9289 7.8648 0.1412 0.9338" +def test_call_module_empty_argument(): + """ + call_module should work if an empty string or an empty list is passed as argument. + """ + with clib.Session() as lib: + lib.call_module("defaults", "") + with clib.Session() as lib: + lib.call_module("defaults", []) + + def test_call_module_invalid_arguments(): """ - Fails for invalid module arguments. + call_module should fail for invalid module arguments. """ with clib.Session() as lib: with pytest.raises(GMTCLibError): @@ -155,7 +176,7 @@ def test_call_module_invalid_arguments(): def test_call_module_invalid_name(): """ - Fails when given bad input. + call_module should fail when given an invalid module name. """ with clib.Session() as lib: with pytest.raises(GMTCLibError): @@ -164,7 +185,7 @@ def test_call_module_invalid_name(): def test_call_module_error_message(): """ - Check is the GMT error message was captured. + Check is the GMT error message was captured when calling a module. """ with clib.Session() as lib: with pytest.raises(GMTCLibError) as exc_info: From 8c9b05816769f99869321c19142dbe5be825b8a0 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Tue, 26 Mar 2024 12:14:02 +0800 Subject: [PATCH 3/8] Fix type hints --- pygmt/clib/session.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pygmt/clib/session.py b/pygmt/clib/session.py index edaaef7997e..62c726d6a12 100644 --- a/pygmt/clib/session.py +++ b/pygmt/clib/session.py @@ -630,6 +630,7 @@ def call_module(self, module: str, args: str | list[str]): ) # 'args' can be (1) a single string or (2) a list of strings. + argv: bytes | ctp.Array[ctp.c_char_p] | None if isinstance(args, str): # 'args' is a single string that contains whitespace-separated arguments. # In this way, we need to correctly handle option arguments that contains From fe84b29076b40f249e416990bdfd07290e06621d Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Tue, 26 Mar 2024 12:28:00 +0800 Subject: [PATCH 4/8] Add one more test to increase code coverage --- pygmt/clib/session.py | 2 ++ pygmt/tests/test_clib.py | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/pygmt/clib/session.py b/pygmt/clib/session.py index 62c726d6a12..50123f9015f 100644 --- a/pygmt/clib/session.py +++ b/pygmt/clib/session.py @@ -620,6 +620,8 @@ def call_module(self, module: str, args: str | list[str]): Raises ------ + GMTInvalidInput + If the ``args`` argument is not a string or a list of strings. GMTCLibError If the returned status code of the function is non-zero. """ diff --git a/pygmt/tests/test_clib.py b/pygmt/tests/test_clib.py index 5f1c3365357..d04508fab09 100644 --- a/pygmt/tests/test_clib.py +++ b/pygmt/tests/test_clib.py @@ -165,6 +165,15 @@ def test_call_module_empty_argument(): lib.call_module("defaults", []) +def test_call_module_invalid_argument_type(): + """ + call_module only accepts a string or a list of strings as module arguments. + """ + with clib.Session() as lib: + with pytest.raises(GMTInvalidInput): + lib.call_module("get", ("FONT_TITLE", "FONT_TAG")) + + def test_call_module_invalid_arguments(): """ call_module should fail for invalid module arguments. From 03070968ae474a61e36cad6d1a9b774c052167b1 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Tue, 26 Mar 2024 21:35:24 +0800 Subject: [PATCH 5/8] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Yvonne Fröhlich <94163266+yvonnefroehlich@users.noreply.github.com> --- pygmt/clib/session.py | 6 +++--- pygmt/tests/test_clib.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pygmt/clib/session.py b/pygmt/clib/session.py index 50123f9015f..fe4a6db566e 100644 --- a/pygmt/clib/session.py +++ b/pygmt/clib/session.py @@ -635,7 +635,7 @@ def call_module(self, module: str, args: str | list[str]): argv: bytes | ctp.Array[ctp.c_char_p] | None if isinstance(args, str): # 'args' is a single string that contains whitespace-separated arguments. - # In this way, we need to correctly handle option arguments that contains + # In this way, we need to correctly handle option arguments that contain # whitepsaces or quotation marks. It's used in PyGMT <= v0.11.0 but is no # longer recommended. mode = self["GMT_MODULE_CMD"] @@ -643,14 +643,14 @@ def call_module(self, module: str, args: str | list[str]): elif isinstance(args, list): # 'args' is a list of strings and each string contains a module argument. # In this way, GMT can correctly handle option arguments with whitespaces or - # quotation marks. This is the preferred way to pass arguments to GMT API + # quotation marks. This is the preferred way to pass arguments to the GMT API # and is used for PyGMT >= v0.12.0. mode = len(args) # 'mode' is the number of arguments. # Pass a null pointer if no arguments are specified. argv = strings_to_ctypes_array(args) if mode != 0 else None else: raise GMTInvalidInput( - "`args' must be either a string or a list of strings." + "'args' must be either a string or a list of strings." ) status = c_call_module(self.session_pointer, module.encode(), mode, argv) diff --git a/pygmt/tests/test_clib.py b/pygmt/tests/test_clib.py index d04508fab09..3707da218fd 100644 --- a/pygmt/tests/test_clib.py +++ b/pygmt/tests/test_clib.py @@ -194,7 +194,7 @@ def test_call_module_invalid_name(): def test_call_module_error_message(): """ - Check is the GMT error message was captured when calling a module. + Check if the GMT error message was captured when calling a module. """ with clib.Session() as lib: with pytest.raises(GMTCLibError) as exc_info: From 88ec919fc9a0db2f20fbb94f0feecf5d9929d149 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Wed, 27 Mar 2024 10:33:25 +0800 Subject: [PATCH 6/8] Fix style issue --- pygmt/clib/session.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pygmt/clib/session.py b/pygmt/clib/session.py index fe4a6db566e..ee0b444b9fb 100644 --- a/pygmt/clib/session.py +++ b/pygmt/clib/session.py @@ -643,8 +643,8 @@ def call_module(self, module: str, args: str | list[str]): elif isinstance(args, list): # 'args' is a list of strings and each string contains a module argument. # In this way, GMT can correctly handle option arguments with whitespaces or - # quotation marks. This is the preferred way to pass arguments to the GMT API - # and is used for PyGMT >= v0.12.0. + # quotation marks. This is the preferred way to pass arguments to the GMT + # API and is used for PyGMT >= v0.12.0. mode = len(args) # 'mode' is the number of arguments. # Pass a null pointer if no arguments are specified. argv = strings_to_ctypes_array(args) if mode != 0 else None From 8ce51bcfbee760a61e8cb508a23cd997477569a5 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Wed, 27 Mar 2024 18:58:17 +0800 Subject: [PATCH 7/8] Update pygmt/tests/test_clib.py Co-authored-by: Michael Grund <23025878+michaelgrund@users.noreply.github.com> --- pygmt/tests/test_clib.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pygmt/tests/test_clib.py b/pygmt/tests/test_clib.py index 3707da218fd..201d7d27fb7 100644 --- a/pygmt/tests/test_clib.py +++ b/pygmt/tests/test_clib.py @@ -185,7 +185,7 @@ def test_call_module_invalid_arguments(): def test_call_module_invalid_name(): """ - call_module should fail when given an invalid module name. + call_module should fail when an invalid module name is given. """ with clib.Session() as lib: with pytest.raises(GMTCLibError): From 9c1e901a443cedd720a3dea39766e440215bcfa0 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Sun, 31 Mar 2024 10:48:09 +0800 Subject: [PATCH 8/8] Update pygmt/clib/session.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Yvonne Fröhlich <94163266+yvonnefroehlich@users.noreply.github.com> --- 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 c6e23101190..a9ea8c65f27 100644 --- a/pygmt/clib/session.py +++ b/pygmt/clib/session.py @@ -636,7 +636,7 @@ def call_module(self, module: str, args: str | list[str]): if isinstance(args, str): # 'args' is a single string that contains whitespace-separated arguments. # In this way, we need to correctly handle option arguments that contain - # whitepsaces or quotation marks. It's used in PyGMT <= v0.11.0 but is no + # whitespaces or quotation marks. It's used in PyGMT <= v0.11.0 but is no # longer recommended. mode = self["GMT_MODULE_CMD"] argv = args.encode()