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.

Sign up for GitHub

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

Jump to bottom

Improve performance by avoiding loading the GMT library repeatedly #2930

Merged
merged 12 commits into from
Jan 2, 2024
Merged

Improve performance by avoiding loading the GMT library repeatedly #2930

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
51f3943
clib: Search and load the GMT library only one time
seisman Dec 28, 2023
713b0e9
[skip ci] Improve comments
seisman Dec 29, 2023
6936f59
Merge branch 'main' into clib/load-libgmt
seisman Dec 29, 2023
f589ff8
Add a unittest
seisman Dec 29, 2023
401a0f8
Apply suggestions from code review
seisman Dec 31, 2023
48c00ae
Add a test to check multiprocessing support
seisman Jan 1, 2024
b31fe19
Mark the test as xfail on Windows
seisman Jan 1, 2024
ce263ec
Merge branch 'main' into clib/load-libgmt
seisman Jan 1, 2024
820e11b
Improve tests
seisman Jan 1, 2024
7a2d96d
Remove the multiprocessing test
seisman Jan 1, 2024
bf6083f
Merge branch 'main' into clib/load-libgmt
seisman Jan 2, 2024
11cb76b
Merge branch 'main' into clib/load-libgmt
seisman Jan 2, 2024
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
Prev Previous commit
Next Next commit
Apply suggestions from code review 
Co-authored-by: Wei Ji <[email protected]>
  • Loading branch information
@seisman @weiji14
seisman and weiji14 committed Dec 31, 2023
commit 401a0f812e1b3c6346aead9ceef6d91dbadbd866
2 changes: 1 addition & 1 deletion pygmt/clib/session.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()


Expand Down
2 changes: 1 addition & 1 deletion pygmt/tests/test_clib_loading.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ def test_brokenlib_brokenlib_workinglib(self):

class TestLibgmtCount:
Copy link
Member

Choose a reason for hiding this comment

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

Maybe better to put this unit test in test_session_management.py?

Copy link
Member Author

Choose a reason for hiding this comment

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

Prefer to keep it in test_clib_loading.py because this unit test is actually not related to session management.

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 kinda wanted to move the test since test_clib_loading.py has 300+ lines of code, while test_session_management.py has <100 lines, and we're kinda checking that Session() doesn't reload libgmt here, but up to you 🙂

"""
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
Expand Down