Skip to content

Move variable __gmt_version__ to pygmt.clib to avoid cyclic-import errors #2713

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

Merged
seisman merged 2 commits into from
Oct 5, 2023
Merged

Move variable __gmt_version__ to pygmt.clib to avoid cyclic-import errors #2713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8a7d55c
Move variable __gmt_version__ to pygmt.clib to avoid cyclic-import er…
seisman Oct 3, 2023
20f21f5
Merge branch 'main' into cyclic-import
seisman Oct 5, 2023
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
4 changes: 0 additions & 4 deletions pygmt/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,9 @@
import sys
from importlib.metadata import version

from pygmt import clib

# Get semantic version through setuptools-scm
__version__ = f'v{version("pygmt")}' # e.g. v0.1.2.dev3+g0ab3cd78
__commit__ = __version__.split("+g")[-1] if "+g" in __version__ else "" # 0ab3cd78
with clib.Session() as lib:
__gmt_version__ = lib.info["version"]
Comment on lines -24 to -30
Copy link
Member

@weiji14 weiji14 Oct 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Looking at how rasterio handles __gdal_version__ at https://github.com/rasterio/rasterio/blob/6bf045cc5e02c9f28e78d48b97b308490207d27d/rasterio/__init__.py#L84-L86, they do:

from rasterio._version import gdal_version

__gdal_version__ = gdal_version()

They have a _version.pyx and a _version.pxd file that seems to be reading from the gdal.h header file. Do you think we could implement something similar for PyGMT/GMT by looking at include/gmt_version.h?

Copy link
Member Author

@seisman seisman Oct 4, 2023

Choose a reason for hiding this comment

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

The way we get the GMT version string doesn't matter. What matters is that the __gmt_version__ variable is defined in pygmt/__init__.py and is internally used in other files.

Take pygmt/src/timestamp.py as an example. It imports the __gmt_version__ variable from pygmt/__init__.py. However, pygmt/__init__.py imports Figure from pygmt/figure.py and pygmt/figure.py imports timestamp from pygmt/src/timestamp.py. Thus, there are always cyclic import issues if we define __gmt_version__ in pygmt/__init__.py.

The only solution I can see is defining __gmt_version__ in another file, like pygmt/clib/__init__.py in this PR, or a new file like pygmt/_version.py.

Copy link
Member

@weiji14 weiji14 Oct 5, 2023

Choose a reason for hiding this comment

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

Ok, let's keep things in pygmt/clib/__init__.py for now since it's all internal variables, to get the Style Checks passing. We can look into moving things later to _version.py if needed.


# Import modules to make the high-level GMT Python API
from pygmt import datasets
Expand Down
3 changes: 3 additions & 0 deletions pygmt/clib/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,6 @@
"""

from pygmt.clib.session import Session

with Session() as lib:
__gmt_version__ = lib.info["version"]
3 changes: 1 addition & 2 deletions pygmt/src/ternary.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
"""
import pandas as pd
from packaging.version import Version
from pygmt import __gmt_version__
from pygmt.clib import Session
from pygmt.clib import Session, __gmt_version__
from pygmt.helpers import build_arg_string, fmt_docstring, kwargs_to_strings, use_alias


Expand Down
3 changes: 1 addition & 2 deletions pygmt/src/timestamp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
import warnings

from packaging.version import Version
from pygmt import __gmt_version__
from pygmt.clib import Session
from pygmt.clib import Session, __gmt_version__
from pygmt.helpers import build_arg_string, is_nonstr_iter

__doctest_skip__ = ["timestamp"]
Expand Down
3 changes: 2 additions & 1 deletion pygmt/tests/test_accessor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
import pytest
import xarray as xr
from packaging.version import Version
from pygmt import __gmt_version__, which
from pygmt import which
from pygmt.clib import __gmt_version__
from pygmt.datasets import load_earth_relief
from pygmt.exceptions import GMTInvalidInput

Expand Down
3 changes: 2 additions & 1 deletion pygmt/tests/test_grdfill.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
import pytest
import xarray as xr
from packaging.version import Version
from pygmt import __gmt_version__, grdfill, load_dataarray
from pygmt import grdfill, load_dataarray
from pygmt.clib import __gmt_version__
from pygmt.exceptions import GMTInvalidInput
from pygmt.helpers import GMTTempFile
from pygmt.helpers.testing import load_static_earth_relief
Expand Down
3 changes: 2 additions & 1 deletion pygmt/tests/test_meca.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
import pandas as pd
import pytest
from packaging.version import Version
from pygmt import Figure, __gmt_version__
from pygmt import Figure
from pygmt.clib import __gmt_version__
from pygmt.exceptions import GMTInvalidInput
from pygmt.helpers import GMTTempFile

Expand Down