Repository navigation
[Experimental] Update ML deps extras - #40531
jrmccluskey wants to merge 3 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #40531 +/- ##
============================================
+ Coverage 56.16% 56.35% +0.19%
Complexity 2288 2288
============================================
Files 1124 1125 +1
Lines 178336 178453 +117
Branches 1489 1489
============================================
+ Hits 100170 100576 +406
+ Misses 75645 75356 -289
Partials 2521 2521
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
/gemini review |
There was a problem hiding this comment.
📋 Review Summary
This PR updates the Python ML test dependency extras in sdks/python/setup.py to re-enable tensorflow_transform (1.21.x), xgboost (>=2.1.3 on Python 3.11–3.12), and google-adk (2.x) alongside Python 3.14 environment markers, while fixing two downstream test compatibility issues with newer XGBoost and NumPy versions. The dependency bounds and test fixes are well-reasoned, and there are a couple of minor cleanup items in setup.py to keep the extra definitions consistent.
🔍 General Feedback
- The test fixes in
xgboost_inference_test.pyandbase_test.pycleanly address attribute persistence across model reloads and NumPy >= 2.4 scalar conversion rules. - In
sdks/python/setup.py, a few related standalone/legacy extras (xgboost,ml_test,p310_ml_test) and duplicatetokenizersentries inml_base_corecould be aligned with the updated dependency bounds.
💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.
💬 Inline Findings
-
sdks/python/setup.py(line 211): 🟢 Low:ml_base_corecurrently specifiestokenizerstwice:'tokenizers<0.23'here on line 211 and'tokenizers>=0.13.3,<0.22.0'on line 217 (which is strictly narrower and makes<0.23redundant). Consider consolidating these into a singletokenizersrequirement entry so the version cap and accompanying comment stay in sync. -
sdks/python/setup.py(line 623): 🟢 Low: Sincepython_requires = '>=3.11'(andtox.inionly definespy311-mlthroughpy314-ml),p310_ml_testappears to be unused. In addition, the standalone'xgboost'extra further down (line 699:'xgboost': ['xgboost>=1.6.0,<2.1.3', 'datatable==1.0.0']) still capsxgboost<2.1.3, which contradicts'xgboost>=2.1.3'added here forp311_ml_testandp312_ml_test. Consider removingp310_ml_testand updating the'xgboost'extra bound to match.
Experimental LLM driven update of our ML dependency extra lists to modernize the suite and get more of these tests running regularly again.
DO NOT MERGE