-
Notifications
You must be signed in to change notification settings - Fork 28k
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
[SPARK-48083][SPARK-48084][ML][TESTS] Remove JIRA comments for reenabling ML compatibility tests #46419
Conversation
Would you mind describing why those skips are legitimate in the PR description? |
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.
Thank you for the clean ups the outdated comments.
@@ -21,7 +21,6 @@ | |||
from pyspark.sql import SparkSession | |||
from pyspark.ml.tests.connect.test_legacy_mode_classification import ClassificationTestsMixin | |||
|
|||
# TODO(SPARK-48083): Reenable this test case |
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.
session.copyFromLocalToFs failure with 3.5 client <> 4.0 server
this is not an issue,
copyFromLocalToFs
requires spark server to config spark.connect.copyFromLocalToFs.allowDestLocal
to False, because the test can only use local fs.
@@ -20,7 +20,6 @@ | |||
from pyspark.sql import SparkSession | |||
from pyspark.ml.tests.connect.test_legacy_mode_evaluation import EvaluationTestsMixin | |||
|
|||
# TODO(SPARK-48084): Reenable this test case |
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.
This test error pyspark.ml.connect.evaluation not working in 3.5 client <> 4.0 server
is caused by cloudpickle
forward incompatibility, it is not related to ML code
Merged to branch-3.5. |
…ling ML compatibility tests ### What changes were proposed in this pull request? Enable spark ml test ### Why are the changes needed? For test_connect_classification.py, session.copyFromLocalToFs failure with 3.5 client <> 4.0 server this is not an issue, copyFromLocalToFs requires spark server to config spark.connect.copyFromLocalToFs.allowDestLocal to False, because the test can only use local fs. For test_connect_evaluation.py, This test error pyspark.ml.connect.evaluation not working in 3.5 client <> 4.0 server is caused by cloudpickle forward incompatibility, it is not related to ML code ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? CI. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #46419 from WeichenXu123/reenable-test-3.5. Authored-by: Weichen Xu <weichen.xu@databricks.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
What changes were proposed in this pull request?
Enable spark ml test
Why are the changes needed?
For test_connect_classification.py,
session.copyFromLocalToFs failure with 3.5 client <> 4.0 server
this is not an issue,
copyFromLocalToFs requires spark server to config spark.connect.copyFromLocalToFs.allowDestLocal to False, because the test can only use local fs.
For test_connect_evaluation.py,
This test error pyspark.ml.connect.evaluation not working in 3.5 client <> 4.0 server is caused by cloudpickle forward incompatibility, it is not related to ML code
Does this PR introduce any user-facing change?
No.
How was this patch tested?
CI.
Was this patch authored or co-authored using generative AI tooling?
No.