Create rule S7191: PySpark "withColumns" should be preferred over "withColumn" when multiple columns are specified #4633
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
thomas-serre-sonarsource
force-pushed
the
rule/add-RSPEC-S7191
branch
2 times, most recently
from
ee30e4c to
e24d4d9
Compare
thomas-serre-sonarsource
requested a review
from guillaume-dequenne-sonarsource
thomas-serre-sonarsource
force-pushed
the
rule/add-RSPEC-S7191
branch
from
e24d4d9 to
5460d72
Compare
rules/S7191/python/rule.adoc
Outdated
| @@ -0,0 +1,65 @@ | |||
| This rule identifies instances where multiple `withColumn` calls are used in succession to add or modify columns in a PySpark DataFrame. It suggests using `withColumns` instead, which is more efficient and concise when dealing with multiple columns. | |||
There was a problem hiding this comment.
Nitpick: Maybe we could drop the final "when dealing with multiple columns.", which can be a bit verbose?
rules/S7191/python/rule.adoc
Outdated
| from pyspark.sql.functions import col | ||
| spark = SparkSession.builder.getOrCreate() | ||
| df = spark.createDataFrame([[1,2],[2,3]], ["id", "value"]) | ||
| df_with_new_cols = df.withColumn("value_plus_1", col("value") + 1).withColumn("value_plus_2", col("value") + 2).withColumn("value_plus_3", col("value") + 3) # Non-compliant |
There was a problem hiding this comment.
Nitpick: for consistency it should beNoncompliant.
There was a problem hiding this comment.
Hum I copy this way of writing it from another PR but can't find which one. Anyway, I follow your suggestion.
rules/S7191/python/rule.adoc
Outdated
| from pyspark.sql.functions import col | ||
| spark = SparkSession.builder.getOrCreate() | ||
| df = spark.createDataFrame([[1,2],[2,3]], ["id", "value"]) | ||
| df_with_new_cols = df.withColumn("value_plus_1", col("value") + 1).withColumn("value_plus_2", col("value") + 2).withColumn("value_plus_3", col("value") + 3) # Non-compliant |
There was a problem hiding this comment.
IMO the examples should be crystal clear, so we shouldn't mix Noncompliant/Compliant cases there.
rules/S7191/python/metadata.json
Outdated
| "MAINTAINABILITY": "MEDIUM", | ||
| "RELIABILITY": "MEDIUM" | ||
| }, | ||
| "attribute": "CONVENTIONAL" |
There was a problem hiding this comment.
What about EFFICIENT here since it's primarily about performances?
thomas-serre-sonarsource
force-pushed
the
rule/add-RSPEC-S7191
branch
from
5460d72 to
af438da
Compare
…umns are specified
thomas-serre-sonarsource
force-pushed
the
rule/add-RSPEC-S7191
branch
from
af438da to
cb5c361
Compare
thomas-serre-sonarsource
requested a review
from guillaume-dequenne-sonarsource
rules/S7191/python/metadata.json
Outdated
| @@ -0,0 +1,26 @@ | |||
| { | |||
| "title": "`withColumns` method should be preferred over `withColumn` when multiple columns are specified", | |||
There was a problem hiding this comment.
I missed it in the first round, but I think that formatted text doesn't actually display in rule titles (as opposed with descriptions), so it's better to use double quotes rather than backticks here.
guillaume-dequenne-sonarsource
changed the title
|
|
You can preview this rule here (updated a few minutes after each push).
Review
A dedicated reviewer checked the rule description successfully for: