[SPARK-28980][CORE][SQL][STREAMING][MLLIB] Remove most items deprecated in Spark 2.2.0 or earlier, for Spark 3 #25684

srowen · 2019-09-04T22:50:31Z

What changes were proposed in this pull request?

Remove SQLContext.createExternalTable and Catalog.createExternalTable, deprecated in favor of createTable since 2.2.0, plus tests of deprecated methods
Remove HiveContext, deprecated in 2.0.0, in favor of SparkSession.builder.enableHiveSupport
Remove deprecated KinesisUtils.createStream methods, plus tests of deprecated methods, deprecate in 2.2.0
Remove deprecated MLlib (not Spark ML) linear method support, mostly utility constructors and 'train' methods, and associated docs. This includes methods in LinearRegression, LogisticRegression, Lasso, RidgeRegression. These have been deprecated since 2.0.0
Remove deprecated Pyspark MLlib linear method support, including LogisticRegressionWithSGD, LinearRegressionWithSGD, LassoWithSGD
Remove 'runs' argument in KMeans.train() method, which has been a no-op since 2.0.0
Remove deprecated ChiSqSelector isSorted protected method
Remove deprecated 'yarn-cluster' and 'yarn-client' master argument in favor of 'yarn' and deploy mode 'cluster', etc

Notes:

I was not able to remove deprecated DataFrameReader.json(RDD) in favor of DataFrameReader.json(Dataset); the former was deprecated in 2.2.0, but, it is still needed to support Pyspark's .json() method, which can't use a Dataset.
Looks like SQLContext.createExternalTable was not actually deprecated in Pyspark, but, almost certainly was meant to be? Catalog.createExternalTable was.
I afterwards noted that the toDegrees, toRadians functions were almost removed fully in SPARK-25908, but Felix suggested keeping just the R version as they hadn't been technically deprecated. I'd like to revisit that. Do we really want the inconsistency? I'm not against reverting it again, but then that implies leaving SQLContext.createExternalTable just in Pyspark too, which seems weird.
I kept LogisticRegressionWithSGD, LinearRegressionWithSGD, LassoWithSGD, RidgeRegressionWithSGD in Pyspark, though deprecated, as it is hard to remove them (still used by StreamingLogisticRegressionWithSGD?) and they are not fully removed in Scala. Maybe should not have been deprecated.

Why are the changes needed?

Deprecated items are easiest to remove in a major release, so we should do so as much as possible for Spark 3. This does not target items deprecated 'recently' as of Spark 2.3, which is still 18 months old.

Does this PR introduce any user-facing change?

Yes, in that deprecated items are removed from some public APIs.

How was this patch tested?

Existing tests.

srowen

Yeah, it's a big change. But the PR description should summarize the key logical parts of the change and help understand the major pieces. There are only about 5 major removals here, they just touch a lot of files.

srowen · 2019-09-04T22:51:08Z

R/pkg/NAMESPACE

              "sumDistinct",
              "tan",
              "tanh",
-              "toDegrees",


@felixcheung to comment on whether we should revisit going ahead and removing these in Spark 3 anyway. We had previously removed them in Pyspark, Scala and kept them in R.

they were deprecated eg. .Deprecated("degrees") in code, I'm ok with removing them

.Deprecated was just added recently though:
https://github.com/apache/spark/pull/22921/files
This was the previous thread:
#22921 (comment)

ah then I"m less sure if we have never release with the deprecation before and go straight to remove
#22921 (comment)

Yep I see the argument @felixcheung . I reopened it because we have a similar situation with SQLContext.createExternalTable, which was deprecated everywhere but Pyspark. Presumably it was also just an oversight. Looks like we're going to remove that. I'd moderately prefer to remove these for consistency, both in the decision and w.r.t. the toDegrees method. Is that persuasive? If you feel fairly strongly about keeping it deprecated, that's OK and I can revert this part.

I'm ok with it. I'd prefer we deprecate it first though. Is there going to be 2.4.5 before 3.0?

(I generally would prefer use friendliness - ie. don't break code - than consistency, as not all language API are consistent, esp. some methods are language specific, and with R is always lagging behind...)

No prob, I reverted this part of the change.

srowen · 2019-09-04T22:51:37Z

core/src/main/scala/org/apache/spark/SparkConf.scala

      }
    }

-    if (contains("spark.master") && get("spark.master").startsWith("yarn-")) {


@vanzin may be interested in the parts that remove support for yarn-client and yarn-cluster masters.

...kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisUtilsPythonHelper.scala

srowen · 2019-09-04T22:53:44Z

mllib/src/main/scala/org/apache/spark/mllib/classification/LogisticRegression.scala

-   * numIterations: 100, regParm: 0.01, miniBatchFraction: 1.0}.
-   */
-  @Since("0.8.0")
-  @deprecated("Use ml.classification.LogisticRegression or LogisticRegressionWithLBFGS", "2.0.0")


Not sure if @jkbradley or @mengxr want to review the removal of MLlib deprecated items, but these have been deprecated a while.

python/pyspark/sql/catalog.py

cloud-fan · 2019-09-05T08:20:08Z

the SQL part LGTM. I think SQLContext.createExternalTable was meant to be deprecated.

python/pyspark/sql/tests/test_context.py

gaborgsomogyi · 2019-09-05T18:15:33Z

The Kinesis part LGTM, though as @srowen mentioned not sure who is interested in.

srowen · 2019-09-05T19:41:54Z

I added removal of HiveContext -- let's see how this goes with tests. I think that's not controversial?

python/pyspark/ml/tests/test_image.py

HyukjinKwon · 2019-09-06T03:47:22Z

I support this PR. Well, I would even support to remove APIs deprecated as of 2.3.0 but can be done separately - I suspect that might be controversial.

HyukjinKwon · 2019-09-06T04:40:55Z

python/pyspark/sql/tests/test_context.py

        cls.hive_available = True
+        cls.spark = None
        try:
            cls.sc._jvm.org.apache.hadoop.hive.conf.HiveConf()


@srowen, seems this code is still needed. SparkSession.builder.enableHiveSupport().getOrCreate() seems not able to directly check if Hive is available or not. I manually tested and just pushed the changes directly into your branch.

HyukjinKwon · 2019-09-06T04:41:42Z

python/pyspark/ml/tests/test_image.py

                lambda: ImageSchema.toImage("a"))


-class ImageFileFormatOnHiveContextTest(PySparkTestCase):


@srowen, this was removed (see #25684 (comment))

srowen · 2019-09-06T13:34:20Z

Thanks @HyukjinKwon ! Looks like we're in good shape. I will leave it open for a bit, pending one more item about R toDegrees above.

… test_context.py

srowen · 2019-09-09T10:14:25Z

Oops, thanks @HyukjinKwon for restoring the commits I lost. I forgot to pull and just rebased on master

SparkQA · 2019-09-09T12:21:33Z

Test build #110344 has finished for PR 25684 at commit 92da887.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

srowen · 2019-09-09T15:19:57Z

Merged to master

…ed in Spark 2.2.0 or earlier, for Spark 3 ### What changes were proposed in this pull request? - Remove SQLContext.createExternalTable and Catalog.createExternalTable, deprecated in favor of createTable since 2.2.0, plus tests of deprecated methods - Remove HiveContext, deprecated in 2.0.0, in favor of `SparkSession.builder.enableHiveSupport` - Remove deprecated KinesisUtils.createStream methods, plus tests of deprecated methods, deprecate in 2.2.0 - Remove deprecated MLlib (not Spark ML) linear method support, mostly utility constructors and 'train' methods, and associated docs. This includes methods in LinearRegression, LogisticRegression, Lasso, RidgeRegression. These have been deprecated since 2.0.0 - Remove deprecated Pyspark MLlib linear method support, including LogisticRegressionWithSGD, LinearRegressionWithSGD, LassoWithSGD - Remove 'runs' argument in KMeans.train() method, which has been a no-op since 2.0.0 - Remove deprecated ChiSqSelector isSorted protected method - Remove deprecated 'yarn-cluster' and 'yarn-client' master argument in favor of 'yarn' and deploy mode 'cluster', etc Notes: - I was not able to remove deprecated DataFrameReader.json(RDD) in favor of DataFrameReader.json(Dataset); the former was deprecated in 2.2.0, but, it is still needed to support Pyspark's .json() method, which can't use a Dataset. - Looks like SQLContext.createExternalTable was not actually deprecated in Pyspark, but, almost certainly was meant to be? Catalog.createExternalTable was. - I afterwards noted that the toDegrees, toRadians functions were almost removed fully in SPARK-25908, but Felix suggested keeping just the R version as they hadn't been technically deprecated. I'd like to revisit that. Do we really want the inconsistency? I'm not against reverting it again, but then that implies leaving SQLContext.createExternalTable just in Pyspark too, which seems weird. - I *kept* LogisticRegressionWithSGD, LinearRegressionWithSGD, LassoWithSGD, RidgeRegressionWithSGD in Pyspark, though deprecated, as it is hard to remove them (still used by StreamingLogisticRegressionWithSGD?) and they are not fully removed in Scala. Maybe should not have been deprecated. ### Why are the changes needed? Deprecated items are easiest to remove in a major release, so we should do so as much as possible for Spark 3. This does not target items deprecated 'recently' as of Spark 2.3, which is still 18 months old. ### Does this PR introduce any user-facing change? Yes, in that deprecated items are removed from some public APIs. ### How was this patch tested? Existing tests. Closes apache#25684 from srowen/SPARK-28980. Lead-authored-by: Sean Owen <[email protected]> Co-authored-by: HyukjinKwon <[email protected]> Signed-off-by: Sean Owen <[email protected]>

srowen commented Sep 4, 2019

View reviewed changes

This comment has been minimized.

Sign in to view

HyukjinKwon reviewed Sep 5, 2019

View reviewed changes

python/pyspark/sql/tests/test_context.py Show resolved Hide resolved

dongjoon-hyun added MLLIB PYSPARK SPARK CORE SQL STRUCTURED STREAMING YARN labels Sep 5, 2019

srowen commented Sep 5, 2019

View reviewed changes

python/pyspark/ml/tests/test_image.py Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

HyukjinKwon reviewed Sep 6, 2019

View reviewed changes

This comment has been minimized.

Sign in to view

srowen added 2 commits September 8, 2019 19:51

Remove most items deprecated in Spark 2.2.0 or earlier, for Spark 3

21b1e2b

Back out R changes for toRadians

23a92ca

srowen force-pushed the SPARK-28980 branch from dc88070 to 23a92ca Compare September 9, 2019 00:59

This comment has been minimized.

Sign in to view

HyukjinKwon added 2 commits September 9, 2019 18:14

Remove ImageFileFormatOnHiveContextTest and fix enbling Hive logic at…

75bf9c4

… test_context.py

Remove unused import at test_image.py as well

2c51696

spark.createTable -> spark.catalog.createTable

92da887

srowen closed this in 6378d4b Sep 9, 2019

zero323 mentioned this pull request Sep 9, 2019

Reflect removals introduced in SPARK-28980 zero323/pyspark-stubs#216

Merged

srowen deleted the SPARK-28980 branch September 27, 2019 04:33

turboFei mentioned this pull request May 5, 2023

Support yarn-client and yarn-cluster for YarnApplicationOperation apache/kyuubi#4786

Closed

3 tasks

		lambda: ImageSchema.toImage("a"))


		class ImageFileFormatOnHiveContextTest(PySparkTestCase):

[SPARK-28980][CORE][SQL][STREAMING][MLLIB] Remove most items deprecated in Spark 2.2.0 or earlier, for Spark 3 #25684

[SPARK-28980][CORE][SQL][STREAMING][MLLIB] Remove most items deprecated in Spark 2.2.0 or earlier, for Spark 3 #25684

Uh oh!

Conversation

srowen commented Sep 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felixcheung Sep 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

cloud-fan commented Sep 5, 2019

Uh oh!

Uh oh!

gaborgsomogyi commented Sep 5, 2019

Uh oh!

srowen commented Sep 5, 2019

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

HyukjinKwon commented Sep 6, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

srowen commented Sep 6, 2019

Uh oh!

This comment has been minimized.

srowen commented Sep 9, 2019

Uh oh!

SparkQA commented Sep 9, 2019

Uh oh!

srowen commented Sep 9, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

srowen commented Sep 4, 2019 •

edited

Loading

felixcheung Sep 8, 2019 •

edited

Loading