-
Notifications
You must be signed in to change notification settings - Fork 234
WIP: Test lowest versions of all required and optional dependencies #3639
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.
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
base: main
Are you sure you want to change the base?
Changes from 11 commits
3a5d6da
f303964
3f6909d
788fc46
4139f4e
1263075
d1fa38d
1c69406
3b7b777
6120bbe
3299229
0584459
7d542a9
4733bda
e24a6c2
52c78fa
d64087e
af01f75
3cb017c
fe0caf2
15da090
170fb09
b1e26da
b079acc
b7814ed
fe20756
2cc4ba2
f54d002
834be64
8ce0755
4540a5d
668cbe1
f5db0ad
0d8ec91
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -33,18 +33,18 @@ dependencies = [ | |||||||||||||||||||||||
| "numpy>=1.24", | ||||||||||||||||||||||||
| "pandas>=2.0", | ||||||||||||||||||||||||
| "xarray>=2023.04", | ||||||||||||||||||||||||
| "netCDF4", | ||||||||||||||||||||||||
| "packaging", | ||||||||||||||||||||||||
| "netCDF4>=1.7", | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
| "packaging>=22.0", | ||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||
| dynamic = ["version"] | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| [project.optional-dependencies] | ||||||||||||||||||||||||
| all = [ | ||||||||||||||||||||||||
| "contextily", | ||||||||||||||||||||||||
| "geopandas", | ||||||||||||||||||||||||
| "IPython", # 'ipython' is not the correct module name. | ||||||||||||||||||||||||
| "pyarrow", | ||||||||||||||||||||||||
| "rioxarray", | ||||||||||||||||||||||||
| "contextily>=1.2", | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
| try: | |
| import contextily | |
| from xyzservices import TileProvider | |
| _HAS_CONTEXTILY = True | |
| except ImportError: | |
| TileProvider = None | |
| _HAS_CONTEXTILY = False |
Xref geopandas/contextily#183, where xyzservices was included as a requirement of contextily (in v1.2.0)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to know that we require contexily>=1.2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pinning to pyarrow>16 here to fix this error:
________ test_to_numpy_pyarrow_array_pyarrow_dtypes_string[string_view] ________
> ???
E KeyError: 'string_view'
pyarrow/types.pxi:5025: KeyError
During handling of the above exception, another exception occurred:
dtype = 'string_view'
@pytest.mark.skipif(not _HAS_PYARROW, reason="pyarrow is not installed")
@pytest.mark.parametrize(
"dtype",
[
None,
"string",
"utf8", # alias for string
"large_string",
"large_utf8", # alias for large_string
"string_view",
],
)
def test_to_numpy_pyarrow_array_pyarrow_dtypes_string(dtype):
"""
Test the _to_numpy function with PyArrow arrays of PyArrow string types.
"""
> array = pa.array(["abc", "defg", "12345"], type=dtype)
../pygmt/tests/test_clib_to_numpy.py:333:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
pyarrow/array.pxi:230: in pyarrow.lib.array
???
pyarrow/types.pxi:5040: in pyarrow.lib.ensure_type
???
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
> ???
E ValueError: No type alias for string_view
pyarrow/types.pxi:5027: ValueErrorCode was added in #3608:
pygmt/pygmt/tests/test_clib_to_numpy.py
Lines 326 to 336 in 97185e8
| "string_view", | |
| ], | |
| ) | |
| def test_to_numpy_pyarrow_array_pyarrow_dtypes_string(dtype): | |
| """ | |
| Test the _to_numpy function with PyArrow arrays of PyArrow string types. | |
| """ | |
| array = pa.array(["abc", "defg", "12345"], type=dtype) | |
| result = _to_numpy(array) | |
| _check_result(result, np.str_) | |
| npt.assert_array_equal(result, array) |
The pyarrow.StringViewArray class was added in apache/arrow#39652 that was released in pyarrow v16.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PyArrow v16.0 was released in April, 2024. Instead of pinning pyarrow>=16.0, we can just skip string_view if pyarrow<v16.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I just tested older versions from pyarrow v12 to v15, and think we can pin to pyarrow>=13 (v13 was released on 24 Aug 2023) if ignoring string_view. With v12, there is another error on datetime.
_________________________________________________________________ test_to_numpy_pyarrow_array_pyarrow_dtypes_date[date64[ms]] __________________________________________________________________
dtype = 'date64[ms]', expected_dtype = 'datetime64[ms]'
@pytest.mark.skipif(not _HAS_PYARROW, reason="pyarrow is not installed")
@pytest.mark.parametrize(
("dtype", "expected_dtype"),
[
pytest.param("date32[day]", "datetime64[D]", id="date32[day]"),
pytest.param("date64[ms]", "datetime64[ms]", id="date64[ms]"),
],
)
def test_to_numpy_pyarrow_array_pyarrow_dtypes_date(dtype, expected_dtype):
"""
Test the _to_numpy function with PyArrow arrays of PyArrow date types.
date32[day] and date64[ms] are stored as 32-bit and 64-bit integers, respectively,
representing the number of days and milliseconds since the UNIX epoch (1970-01-01).
Here we explicitly check the dtype and date unit of the result.
"""
data = [
date(2024, 1, 1),
datetime(2024, 1, 2),
datetime(2024, 1, 3),
]
array = pa.array(data, type=dtype)
result = _to_numpy(array)
_check_result(result, np.datetime64)
> assert result.dtype == expected_dtype # Explicitly check the date unit.
E AssertionError: assert dtype('<M8[D]') == 'datetime64[ms]'
E + where dtype('<M8[D]') = array(['2024-01-01', '2024-01-02', '2024-01-03'], dtype='datetime64[D]').dtype
../pygmt/tests/test_clib_to_numpy.py:364: AssertionErrorThe main change appears to be in apache/arrow#33321, when pyarrow supported preserving the datetime64 temporal resolution.
Maybe we can keep the string_view test, but add a skip_if(pyarrow.__version__ < 16) marker or something like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting this test failure on windows-2022 at https://github.com/GenericMappingTools/pygmt/actions/runs/15524973929/job/43703740279?pr=3639#step:7:1095:
_______________ [doctest] pygmt.datasets.tile_map.load_tile_map _______________
118 >>> raster.sizes
119 Frozen({'band': 3, 'y': 256, 'x': 512})
120 >>> raster.coords # doctest: +ELLIPSIS, +NORMALIZE_WHITESPACE
121 Coordinates:
122 * band (band) uint8... 1 2 3
123 * y (y) float64... -7.081e-10 -7.858e+04 ... -1.996e+07 -2.004e+07
124 * x (x) float64... -2.004e+07 -1.996e+07 ... 1.996e+07 2.004e+07
125 spatial_ref int... 0
126 >>> # CRS is set only if rioxarray is available
127 >>> if hasattr(raster, "rio"):
Expected:
'EPSG:3857'
Got:
'PROJCS["WGS 84 / Pseudo-Mercator",GEOGCS["WGS 84",DATUM["WGS_1984",SPHEROID["WGS 84",6378137,298.257223563,AUTHORITY["EPSG","7030"]],AUTHORITY["EPSG","6326"]],PRIMEM["Greenwich",0,AUTHORITY["EPSG","8901"]],UNIT["degree",0.0174532925199433,AUTHORITY["EPSG","9122"]],AUTHORITY["EPSG","4326"]],PROJECTION["Mercator_1SP"],PARAMETER["central_meridian",0],PARAMETER["scale_factor",1],PARAMETER["false_easting",0],PARAMETER["false_northing",0],UNIT["metre",1],AXIS["Easting",EAST],AXIS["Northing",NORTH],EXTENSION["PROJ4","+proj=merc +a=6378137 +b=6378137 +lat_ts=0 +lon_0=0 +x_0=0 +y_0=0 +k=1 +units=m +nadgrids=@null +wktext +no_defs"],AUTHORITY["EPSG","3857"]]'
D:\a\pygmt\pygmt\pygmt\datasets\tile_map.py:127: DocTestFailureSimilar situation to the one reported at #3575. We could perhaps pin to a rioxarray version that requires rasterio>=1.4.2? E.g. rioxarray=0.19.0 pins to rasterio>=1.4.3 (corteva/rioxarray#850). However, that version was released too recently on 22 Apr 2025 (<2 months).
Suggestions on what to do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The actual error messages are
ERROR 1: PROJ: proj_create_from_database: C:\Users\runneradmin\micromamba\envs\pygmt/Library/share/proj\proj.db contains DATABASE.LAYOUT.VERSION.MINOR = 3 whereas a number >= 4 is expected. It comes from another PROJ installation.
ERROR 1: PROJ: proj_identify: C:\Users\runneradmin\micromamba\envs\pygmt/Library/share/proj\proj.db contains DATABASE.LAYOUT.VERSION.MINOR = 3 whereas a number >= 4 is expected. It comes from another PROJ installation.
Some related discussions are in corteva/rioxarray#447. I'm unsure if pin rioxarray>=0.19.0 can solve the issue, but please give it a try first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm you're right, got the same ERROR 1: PROJ: ... message even with rioxarray>0.19.0. Probably because we are using uv/PyPI installed packages with incompatible PROJ versions, instead of conda-forge packages where packages are compiled with the same PROJ lib.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dependency tree GMT->GDAL->PROJ installs PROJ 9.4.0, while uv/pypi installs pyproj 3.7.1 which contains PROJ 9.5.1.
Perhaps we can install pyproj via micromamba instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we can install pyproj via micromamba instead?
Tried at commit 4540a5d, same error π’
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if setting env PROJ_WHEEL=YES?
xref: https://pyproj4.github.io/pyproj/dev/installation.html#setup-pyproj
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PROJ_WHEEL seems to be relevant only when building the pyproj wheel, but since we're pulling a compiled version of pyproj from PyPI/conda-forge, it probably doesn't do anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using
--resolution lowest-directfor now, because--resolution lowestgrabs many other transitive dependencies that may be hard to install (e.g. missing wheels for newer Python versions, so requires compilation from source). We could switch to--resolution lowestonce the Python ecosystem does lower bound pins a bit more thoroughly (which may take years).