tvalentyn commented on code in PR #40694:
URL: https://github.com/apache/beam/pull/40694#discussion_r4331457622


##########
sdks/python/setup.py:
##########
@@ -166,6 +166,19 @@ def cythonize(*args, **kwargs):
 
 milvus_dependency = ['pymilvus>=2.5.10,<3.0.0']
 qdrant_dependency = ['qdrant-client>=1.15.0']
+# NOTE: When updating these bounds, also update the bounds in
+# apache_beam/ml/inference/xgboost_tests_requirements.txt, which is used by the
+# XGBoost RunInference integration tests, and the py311-xgboost-* environments
+# in tox.ini.
+xgboost_dependency = [
+    # xgboost<2.0 wheels fail `pip check` on recent pip ("not supported on this
+    # platform"), see https://github.com/apache/beam/issues/31285.

Review Comment:
   consider pinning a specific comment if this is needed. 



##########
sdks/python/setup.py:
##########
@@ -166,6 +166,19 @@ def cythonize(*args, **kwargs):
 
 milvus_dependency = ['pymilvus>=2.5.10,<3.0.0']
 qdrant_dependency = ['qdrant-client>=1.15.0']
+# NOTE: When updating these bounds, also update the bounds in
+# apache_beam/ml/inference/xgboost_tests_requirements.txt, which is used by the
+# XGBoost RunInference integration tests, and the py311-xgboost-* environments
+# in tox.ini.
+xgboost_dependency = [
+    # xgboost<2.0 wheels fail `pip check` on recent pip ("not supported on this
+    # platform"), see https://github.com/apache/beam/issues/31285.
+    'xgboost>=2.0.0,<4',
+    # apache_beam.ml.inference.xgboost_inference imports datatable directly.
+    # 1.1.0 is the first release with Python 3.10+ wheels; no release has
+    # wheels for Python 3.13+.
+    'datatable>=1.1.0,<2',

Review Comment:
   Given how outdated datatable is, i think the XGBoost MH variation using 
datatable is no longer relevant; I think we should not make it a mandatory dep 
for XGBoost users, given there are other MHs that don't need it, and perhaps we 
should outright remove it due to lack of datatable releases.  
   
   



##########
CHANGES.md:
##########
@@ -93,6 +93,7 @@
 * (Python) `Regex.find_all` with `group=Regex.ALL` now returns every group, as 
`(match, group1, group2, ...)`, instead of only group 1, and with 
`outputEmpty=False` it drops a match only when the whole match is empty, 
matching the `group=0` behavior 
([#40399](https://github.com/apache/beam/pull/40399)).
 * (Python) `Regex.all_matches` now returns every group of the regex, instead 
of failing with a `TypeError` when the regex has no groups and dropping nested 
or unmatched trailing groups 
([#40562](https://github.com/apache/beam/issues/40562)).
 * (Python) `--dataflow_endpoint` now defaults to `''` instead of 
`https://dataflow.googleapis.com`. When unset, the Google Cloud client library 
resolves the Dataflow endpoint and honors `GOOGLE_CLOUD_UNIVERSE_DOMAIN`, so 
Dataflow jobs can be submitted in non-default universes. Code that reads 
`GoogleCloudOptions.dataflow_endpoint` should handle an empty value 
([#40569](https://github.com/apache/beam/issues/40569)).
+* (Python) The `xgboost` extras now install `xgboost>=2.0.0,<4` and 
`datatable>=1.1.0,<2`, moving both the lower and upper bounds for both. The 
previous minimum `xgboost` and `datatable` versions (`1.6.0` and `1.0.0` 
respectively) are no longer allowed 
([#40694](https://github.com/apache/beam/issues/40694)).

Review Comment:
   we should update xgboost inference docstring which currently says:
   
   ```
       **Supported Versions:** RunInference APIs in Apache Beam have been tested
       with XGBoost 1.6.0 and 1.7.0
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to