From 51f3943cd6481fcf49c216b2cb00e6c6a696ca06 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Thu, 28 Dec 2023 21:59:58 +0800 Subject: [PATCH 1/8] clib: Search and load the GMT library only one time --- pygmt/clib/session.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pygmt/clib/session.py b/pygmt/clib/session.py index 1f338851a9a..b6a7f4c7bb6 100644 --- a/pygmt/clib/session.py +++ b/pygmt/clib/session.py @@ -87,6 +87,9 @@ np.datetime64: "GMT_DATETIME", } +# load the GMT library outside the Session class so it's loaded once. +_libgmt = load_libgmt() + class Session: """ @@ -308,7 +311,7 @@ def get_libgmt_func(self, name, argtypes=None, restype=None): ._FuncPtr'> """ if not hasattr(self, "_libgmt"): - self._libgmt = load_libgmt() + self._libgmt = _libgmt function = getattr(self._libgmt, name) if argtypes is not None: function.argtypes = argtypes From 713b0e94c41db6a005c275d612c4af92c7906602 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Fri, 29 Dec 2023 21:34:01 +0800 Subject: [PATCH 2/8] [skip ci] Improve comments --- 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 b6a7f4c7bb6..77d8a2bd54a 100644 --- a/pygmt/clib/session.py +++ b/pygmt/clib/session.py @@ -87,7 +87,7 @@ np.datetime64: "GMT_DATETIME", } -# load the GMT library outside the Session class so it's loaded once. +# Load the GMT library outside the Session class to avoid repeating loading. _libgmt = load_libgmt() From f589ff8b687c9fc35d516175ae731dcf1d64d490 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Fri, 29 Dec 2023 22:34:24 +0800 Subject: [PATCH 3/8] Add a unittest --- pygmt/tests/test_clib_loading.py | 40 ++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/pygmt/tests/test_clib_loading.py b/pygmt/tests/test_clib_loading.py index 460f8e68a47..912b2e71ac1 100644 --- a/pygmt/tests/test_clib_loading.py +++ b/pygmt/tests/test_clib_loading.py @@ -11,6 +11,7 @@ import pytest from pygmt.clib.loading import check_libgmt, clib_full_names, clib_names, load_libgmt +from pygmt.clib.session import Session from pygmt.exceptions import GMTCLibError, GMTCLibNotFoundError, GMTOSError @@ -208,6 +209,45 @@ def test_brokenlib_brokenlib_workinglib(self): assert check_libgmt(load_libgmt(lib_fullnames=lib_fullnames)) is None +class TestLibgmtCount: + """ + Test that the GMT library is not repeatly loaded in every session. + """ + + loaded_libgmt = load_libgmt() # Load the GMT library and reuse it when necessary + counter = 0 # Count how many times ctypes.CDLL is called + + def _mock_ctypes_cdll_return(self, libname): # noqa: ARG002 + """ + Mock ctypes.CDLL to count how many times the function is called. + + If ctypes.CDLL is called, the counter increases by one. + """ + self.counter += 1 # Increase the counter + return self.loaded_libgmt + + @pytest.fixture() + def _mock_ctypes(self, monkeypatch): + monkeypatch.setattr(ctypes, "CDLL", self._mock_ctypes_cdll_return) + + @pytest.mark.usefixtures("_mock_ctypes") + def test_libgmt_load_counter(self): + """ + Make sure that the GMT library is not loaded in every session. + """ + with Session() as lib: + _ = lib + with Session() as lib: + _ = lib + assert self.counter == 0 # ctypes.CDLL is not called after two sessions. + + # Explicitly calling load_libgmt to make sure the mock function is correct + load_libgmt() + assert self.counter == 1 + load_libgmt() + assert self.counter == 2 + + ############################################################################### # Test clib_full_names @pytest.fixture(scope="module", name="gmt_lib_names") From 401a0f812e1b3c6346aead9ceef6d91dbadbd866 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Sun, 31 Dec 2023 23:11:27 +0800 Subject: [PATCH 4/8] Apply suggestions from code review Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com> --- pygmt/clib/session.py | 2 +- pygmt/tests/test_clib_loading.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pygmt/clib/session.py b/pygmt/clib/session.py index 77d8a2bd54a..9ae5b980e17 100644 --- a/pygmt/clib/session.py +++ b/pygmt/clib/session.py @@ -87,7 +87,7 @@ np.datetime64: "GMT_DATETIME", } -# Load the GMT library outside the Session class to avoid repeating loading. +# Load the GMT library outside the Session class to avoid repeated loading. _libgmt = load_libgmt() diff --git a/pygmt/tests/test_clib_loading.py b/pygmt/tests/test_clib_loading.py index 912b2e71ac1..14a0ada0850 100644 --- a/pygmt/tests/test_clib_loading.py +++ b/pygmt/tests/test_clib_loading.py @@ -211,7 +211,7 @@ def test_brokenlib_brokenlib_workinglib(self): class TestLibgmtCount: """ - Test that the GMT library is not repeatly loaded in every session. + Test that the GMT library is not repeatedly loaded in every session. """ loaded_libgmt = load_libgmt() # Load the GMT library and reuse it when necessary From 48c00aee65d9a66ab94770ea6fb7bca6a1ece4d5 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Mon, 1 Jan 2024 14:17:59 +0800 Subject: [PATCH 5/8] Add a test to check multiprocessing support --- pygmt/tests/test_session_management.py | 33 ++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/pygmt/tests/test_session_management.py b/pygmt/tests/test_session_management.py index 079c2c4e02c..5ab778f00a7 100644 --- a/pygmt/tests/test_session_management.py +++ b/pygmt/tests/test_session_management.py @@ -2,6 +2,7 @@ Test the session management modules. """ import os +from pathlib import Path import pytest from pygmt.clib import Session @@ -57,3 +58,35 @@ def test_gmt_compat_6_is_applied(capsys): # Make sure no global "gmt.conf" in the current directory assert not os.path.exists("gmt.conf") begin() # Restart the global session + + +def gmt_func(figname): + """ + Workaround to let PyGMT support multiprocessing. + + See + https://github.com/GenericMappingTools/pygmt/issues/217#issuecomment-754774875 + """ + from importlib import reload + + import pygmt + + reload(pygmt) + fig = pygmt.Figure() + fig.basemap(region=[10, 70, -3, 8], projection="X8c/6c", frame="afg") + fig.savefig(figname) + + +def test_session_multiprocessing(): + """ + Make sure that the multiprocessing is supported if pygmt is re-imported. + """ + + import multiprocessing as mp + + fig_prefix = "test_session_multiprocessing" + with mp.Pool(2) as p: + p.map(gmt_func, [f"{fig_prefix}-1.png", f"{fig_prefix}-2.png"]) + for i in [1, 2]: + assert Path(f"{fig_prefix}-{i}.png").exists() + Path(f"{fig_prefix}-{i}.png").unlink() From b31fe1942eb45ccd4ea18c5259dc5f57f2c9c641 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Mon, 1 Jan 2024 14:50:07 +0800 Subject: [PATCH 6/8] Mark the test as xfail on Windows --- pygmt/tests/test_session_management.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pygmt/tests/test_session_management.py b/pygmt/tests/test_session_management.py index 5ab778f00a7..da8117789e2 100644 --- a/pygmt/tests/test_session_management.py +++ b/pygmt/tests/test_session_management.py @@ -2,6 +2,7 @@ Test the session management modules. """ import os +import sys from pathlib import Path import pytest @@ -77,9 +78,13 @@ def gmt_func(figname): fig.savefig(figname) +@pytest.mark.xfail( + condition=sys.platform == "win32", + reason="The session path uses PID of parent process on Windows", +) def test_session_multiprocessing(): """ - Make sure that the multiprocessing is supported if pygmt is re-imported. + Make sure that multiprocessing is supported if pygmt is re-imported. """ import multiprocessing as mp From 820e11b228cabb360ed2bdd1b5b175c85ccdf92a Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Mon, 1 Jan 2024 15:10:30 +0800 Subject: [PATCH 7/8] Improve tests --- pygmt/tests/test_clib_loading.py | 13 ++++++------- pygmt/tests/test_session_management.py | 5 ++--- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/pygmt/tests/test_clib_loading.py b/pygmt/tests/test_clib_loading.py index 14a0ada0850..cd4a5081a5a 100644 --- a/pygmt/tests/test_clib_loading.py +++ b/pygmt/tests/test_clib_loading.py @@ -215,7 +215,7 @@ class TestLibgmtCount: """ loaded_libgmt = load_libgmt() # Load the GMT library and reuse it when necessary - counter = 0 # Count how many times ctypes.CDLL is called + counter = 0 # Global counter for how many times ctypes.CDLL is called def _mock_ctypes_cdll_return(self, libname): # noqa: ARG002 """ @@ -226,15 +226,14 @@ def _mock_ctypes_cdll_return(self, libname): # noqa: ARG002 self.counter += 1 # Increase the counter return self.loaded_libgmt - @pytest.fixture() - def _mock_ctypes(self, monkeypatch): - monkeypatch.setattr(ctypes, "CDLL", self._mock_ctypes_cdll_return) - - @pytest.mark.usefixtures("_mock_ctypes") - def test_libgmt_load_counter(self): + def test_libgmt_load_counter(self, monkeypatch): """ Make sure that the GMT library is not loaded in every session. """ + # Monkeypatch the ctypes.CDLL function + monkeypatch.setattr(ctypes, "CDLL", self._mock_ctypes_cdll_return) + + # Create two sessions and check the global counter with Session() as lib: _ = lib with Session() as lib: diff --git a/pygmt/tests/test_session_management.py b/pygmt/tests/test_session_management.py index da8117789e2..13eec5ca174 100644 --- a/pygmt/tests/test_session_management.py +++ b/pygmt/tests/test_session_management.py @@ -63,10 +63,9 @@ def test_gmt_compat_6_is_applied(capsys): def gmt_func(figname): """ - Workaround to let PyGMT support multiprocessing. + Workaround for multiprocessing support in PyGMT. - See - https://github.com/GenericMappingTools/pygmt/issues/217#issuecomment-754774875 + https://github.com/GenericMappingTools/pygmt/issues/217#issuecomment-754774875. """ from importlib import reload From 7a2d96de46158edb4580bccba55f43271569fb04 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Tue, 2 Jan 2024 01:01:11 +0800 Subject: [PATCH 8/8] Remove the multiprocessing test --- pygmt/tests/test_session_management.py | 37 -------------------------- 1 file changed, 37 deletions(-) diff --git a/pygmt/tests/test_session_management.py b/pygmt/tests/test_session_management.py index 13eec5ca174..079c2c4e02c 100644 --- a/pygmt/tests/test_session_management.py +++ b/pygmt/tests/test_session_management.py @@ -2,8 +2,6 @@ Test the session management modules. """ import os -import sys -from pathlib import Path import pytest from pygmt.clib import Session @@ -59,38 +57,3 @@ def test_gmt_compat_6_is_applied(capsys): # Make sure no global "gmt.conf" in the current directory assert not os.path.exists("gmt.conf") begin() # Restart the global session - - -def gmt_func(figname): - """ - Workaround for multiprocessing support in PyGMT. - - https://github.com/GenericMappingTools/pygmt/issues/217#issuecomment-754774875. - """ - from importlib import reload - - import pygmt - - reload(pygmt) - fig = pygmt.Figure() - fig.basemap(region=[10, 70, -3, 8], projection="X8c/6c", frame="afg") - fig.savefig(figname) - - -@pytest.mark.xfail( - condition=sys.platform == "win32", - reason="The session path uses PID of parent process on Windows", -) -def test_session_multiprocessing(): - """ - Make sure that multiprocessing is supported if pygmt is re-imported. - """ - - import multiprocessing as mp - - fig_prefix = "test_session_multiprocessing" - with mp.Pool(2) as p: - p.map(gmt_func, [f"{fig_prefix}-1.png", f"{fig_prefix}-2.png"]) - for i in [1, 2]: - assert Path(f"{fig_prefix}-{i}.png").exists() - Path(f"{fig_prefix}-{i}.png").unlink()