Modernize dependencies and CI, add Python 3.13 support, and fix NumPy 2 compatibility issues#372
Conversation
…ydantic version deprecation warning
…bar for compatibility with py3.13 and py 3.14
… - corrections made
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #372 +/- ##
==========================================
+ Coverage 82.82% 83.36% +0.54%
==========================================
Files 34 39 +5
Lines 2241 2567 +326
==========================================
+ Hits 1856 2140 +284
- Misses 385 427 +42 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
codecov is comparing against an outdated base commit ( |
| return np.str_ | ||
| if np.isscalar(obj): | ||
| return np.obj2sctype(type(obj)) | ||
| return np.asarray(obj).dtype.type |
There was a problem hiding this comment.
https://numpy.org/doc/2.3/numpy_2_0_migration_guide.html says use np.dtype(obj).type instead of obj2sctype, and NumPy 1.25 seem to contain np.dtype (https://numpy.org/doc/1.25/reference/arrays.dtypes.html). Why is all the extra logic required?
There was a problem hiding this comment.
Thanks @pjonsson, it's a good point. I simplified this while keeping separate scalar and dtype-bearing paths. np.dtype(obj).type is not safe for all current callers: it raises for values like 1, 1.0, and ndarray inputs, and string values can be interpreted as dtype aliases. Scalars now use np.dtype(type(obj)).type, while dtype-bearing objects use np.dtype(obj.dtype).type.
| idxs = eq.argmax(axis=axis) | ||
| mask = ~np.array(eq.any(axis=axis)) | ||
|
|
||
| # Fix: np.isnan crashes on strings, None, lists, etc. |
There was a problem hiding this comment.
I find it difficult to sort out what fixes that are done because of Python 3.13/3.14 incompatibilities, NumPy 2 incompatibilities, or CI incompatibilities. Is it a NumPy 2 problem that np.isnan crashes on anything that isn't array_like?
There was a problem hiding this comment.
agreed! that this needed clearer separation, this is not specifically a NumPy 2 issue; np.isnan is only defined for numeric-like values, so the old unconditional check was too broad. I updated the comment and kept non-numeric search values on the normal equality/not-found path.
Does code coverage work for main, but CodeCov was down when the last commit was merged to main? If so, merging #358 and rebasing this on top of latest main should get the right coverage information. And if coverage doesn't work on main, perhaps the commit that fixes coverage in this PR can be cherry-picked into a separate branch and submitted as a separate PR that can be merged first to establish the baseline coverage? |
| shell: bash -l {0} | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - uses: actions/checkout@v5 |
There was a problem hiding this comment.
Is there a reason to not use version 6?
There was a problem hiding this comment.
there was no reason to avoid version 6, so actions/checkout is now updated to v6.
| POETRY_VERSION: 1.5.1 | ||
|
|
||
| POETRY_VERSION: 2.3.3 | ||
| FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true # fixes the Node.js 20 warning |
There was a problem hiding this comment.
It's nice if the comment contains a note about which action that triggers "the Node.js 20 warning" so this variable can be removed when that action is updated in the future.
There was a problem hiding this comment.
that makes sense. addressed this by removing the override instead of documenting it. The workflow now uses actions/checkout@v6 and setup-miniconda@v4, so FORCE_JAVASCRIPT_ACTIONS_TO_NODE24 is no longer needed.
|
|
||
| - repo: https://github.com/python-poetry/poetry | ||
| - repo: https://github.com/python-poetry/poetry | ||
| rev: 1.5.1 |
There was a problem hiding this comment.
The POETRY_VERSION in the CI is set to 2.3.3 by this PR, should the version here be the same?
There was a problem hiding this comment.
nice catch, yes, it should match, addressed this. The poetry-check pre-commit hook now uses 2.3.3 to align with POETRY_VERSION in CI.
| return data | ||
| if len(x[valid]) < 2: | ||
| return data | ||
| data = data.copy() # avoid mutating the caller's array |
There was a problem hiding this comment.
This looks like it changes both behavior of the interp function and how expensive it is to call it.
There was a problem hiding this comment.
agreed, it indeed makes things expensive at this point. addressed in the latest commit: e91ab28. The previous lazy Dask path could process chunks independently and change interpolation results at chunk boundaries. I changed the 1D Dask path to rechunk to one block before map_blocks, preserving the eager/global interpolation behavior while still delaying execution until compute time. I also added a multi-chunk regression test for this.
| try: | ||
| from xgboost import dask as dxgb | ||
| except ImportError as e: | ||
| raise ImportError("xgboost[dask] is required for predict_random_forest.") from e |
There was a problem hiding this comment.
It looks like the intention is to replace the import error generated by the system, shouldn't this raise from None instead to avoid showing a bunch of extra stuff in the backtrace?
There was a problem hiding this comment.
thanks, agreed. addressed in e91ab28. This now raises the clearer ImportError with from None, so the original import traceback is suppressed.
| try: | ||
| from xgboost import dask as dxgb | ||
| except ImportError as e: | ||
| raise ImportError( |
There was a problem hiding this comment.
It looks like the intention is to replace the import error generated by the system, shouldn't this raise from None instead to avoid showing a bunch of extra stuff in the backtrace?
There was a problem hiding this comment.
this now also raises with from None, matching the intended replacement error behavior.
@pjonsson I did not make a branch-level code change for this because it appears to be a baseline/main coverage upload issue rather than something this PR branch can fully fix. After e91ab28, Codecov is comparing against the pushed head and patch coverage improved, but it still reports the base as 010c214 and 38 commits behind main. Rebasing onto a main commit with a valid coverage upload should make the comparison meaningful. @pjonsson thanks again for your time and efforts for reviewing this PR I pushed e91ab28 to address the review feedback from @pjonsson . |
…ompat Backport changes from openeo-processes-dask PR Open-EO#372: - Bump python to >=3.10,<3.14, add py3.13 classifier - Allow numpy >=1.26.3,<3; pystac >=1.8.0 - Remove upper bounds from xarray, dask, geopandas, dask-geopandas, xgboost - Add gdal >=3.9 optional dep from conda-forge - Switch CI to conda-incubator/setup-miniconda, Poetry 2.3.3, actions/checkout@v6 - Replace np.obj2sctype and np.issubsctype for NumPy 2 compat - Fix array_find (filled arrays, no warning), array_interpolate_linear (lazy dask) - Fix random_forest xgboost import (dxgb), resample tie handling, zero-size dim check - Update tests: model_validate, new array_find/interpolate/ml tests
clausmichele
changed the title
- Restructure README installation: conda first (mirrors CI), then system packages - Replace auto-detecting PyPI badge with explicit Python 3.10-3.13 badge - Fix codecov badge to point to correct org and branch - Remove non-existent 'experimental' extra from extras list - Clarify which extras depend on GDAL (implementations only) - Add install-methods.yml CI workflow to test conda+pip, micromamba+pip, and system packages+pip installation paths weekly
…puts - conda-pip: mamba not available in PATH, use conda install instead - micromamba-pip: remove invalid inputs (channels, channel-priority, conda-remove-defaults) not accepted by setup-micromamba@v2, add required environment-name - Cannot merge into a single matrix job because uses: does not support dynamic expressions
3 participants
This PR modernises
openeo-processes-daskdependencies and CI, extends Python support to 3.13, restores compatibility with current versions of NumPy, XGBoost, and pystac, and fixes a set of runtime and test issues uncovered during the upgrade work. Removes the xcube-eopf dependency and EOPF data support inload_stac.The main goals are:
array_find,array_interpolate_linear,eq,resample_cube_temporal, andrandom_forestDependency and packaging changes
Python support
>=3.10,<3.13to>=3.10,<3.14Package version
openeo-processes-dask:2025.10.1→2026.06.2Dependency policy
This PR follows a “widen, don’t shift” approach: upper bounds are removed where possible to allow newer versions, while existing lower bounds are largely preserved to avoid unnecessary disruption for older environments.
Updated constraints
numpy<2.0.0>=1.26.3,<3xarray>=2022.11.0,<2025.08.01>=2022.11.0dask[array,dataframe,distributed]>=2023.4.0,<2025.2.0>=2023.4.0xgboost>=1.5.1,<2.1.4>=1.5.1pystac<1.12.0>=1.8.0geopandas>=0.11.1,<1>=0.11.1dask-geopandas0.4.3>=0.4.3pyarrow^15.0.2>=15.0.2openeo-pg-parser-networkx>=2025.10>=2025.10.0New optional dependency
gdal >=3.9added as an explicit optional dependency, installed from conda-forge in CICI and release workflow changes
The CI and release workflows were reworked to install GDAL via conda-forge instead of the UbuntuGIS PPA.
Main changes
.github/ci-environment.ymlto define the CI conda environmentvirtualenvs.create false1.5.1to2.3.3bashtobash -l {0}for conda activation.venvcache strategy with separate caches for conda packages and pip wheelspoetry installstep in the release workflowpoetry runThis removes a fragile CI path that depended on matching the Python GDAL binding to the system GDAL version at runtime.
New file
.github/ci-environment.ymlCode fixes
NumPy 2 compatibility
utils.py—get_scalar_type()Replaced
np.obj2sctype()with logic based onnp.asarray(obj).dtype.typeandnp.dtype(obj.dtype).type, which works on both NumPy 1.x and 2.x.Also added a
strfast path so string inputs are handled consistently.comparison.py—np.issubsctype()removalReplaced all uses of
np.issubsctype()withnp.issubdtype().In
eq(), the incompatible-type branch now returns a boolean array matching input shape instead of a scalarFalse, avoiding shape-related issues for array inputs.arrays.pyfixesarray_find()Reworked
array_find()to fix three issues:np.isnan(value)could fail on non-numeric valuesThe new behaviour is:
For not-found positions, the function now returns filled values (
999999) instead of masked entries.array_interpolate_linear()Fixed the Dask 1D path so it stays lazy by using
da.map_blocks(...).Also:
valid.all()logicrandom_forest.pyModernised XGBoost Dask usage by switching from
xgb.dask.*access throughimport xgboost as xgbto:Also added
ImportErrorguards with clearer error messages.cubes/resample.pyresample_cube_temporal()Simplified tie handling when multiple timestamps are equally close to the target.
The old implementation only handled specific
argwhere()output shapes. The new implementation flattens the index result and always takes the first match, which consistently selects the earliest timestamp and avoids crashes on other tie configurations.cubes/apply_neighborhood_intertwin.pyAdded an explicit zero-size dimension check and now raises a clear
ValueErrorwhen an input dimension has size 0.Test and compatibility updates
Pydantic v2
Replaced all
parse_obj()uses withmodel_validate()across fixtures and tests.Test fixes
.compute()array_find()string not-found expectation from99999to999999The previous string sentinel test was not validating the actual returned value because masked positions were effectively being ignored.
Behavioural changes to note
These are the main observable behaviour changes introduced by the fixes:
numpy >=1.26.3is now required, and NumPy 2.x is supported.pystac >=1.8.0is now required.array_find()now returns fillednp.ndarray/da.Arrayresults instead of masked arrays. Not-found positions are represented by999999.array_interpolate_linear()now preserves laziness for 1D Dask inputs instead of forcing eager behaviour.eq()now returns a shape-preserving boolean array for incompatible array/type comparisons instead of a scalarFalse.CI now assumes Poetry 2.x compatibility.
Testing notes
eq()andarray_find()