SONARPY-2496 Create rule S7193 PySpark DataFrame toPandas function should be avoided #4636
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
guillaume-dequenne-sonarsource
force-pushed
the
rule/add-RSPEC-S7193
branch
from
6158632 to
ef2c14b
Compare
guillaume-dequenne-sonarsource
changed the title
guillaume-dequenne-sonarsource
force-pushed
the
rule/add-RSPEC-S7193
branch
3 times, most recently
from
c4cfd72 to
6f752eb
Compare
…should be avoided
guillaume-dequenne-sonarsource
force-pushed
the
rule/add-RSPEC-S7193
branch
from
6f752eb to
17a753a
Compare
There was a problem hiding this comment.
This looks really good to me! I have just left some small comments. For the implementation part I think it would be wise to only raise when we know the DF is loaded from a file parquet, csv etc... and still keep the exception in place (filtering, usages in visualization tools) to remove FPs.
rules/S7193/python/rule.adoc
Outdated
|
|
||
| For this reason, it is generally advisable to avoid using `toPandas` unless you are certain that the dataset is small enough to be handled comfortably by a single machine. Instead, consider using Spark's built-in functions and capabilities to perform data processing tasks in a distributed manner. | ||
|
|
||
| If conversion to Pandas is necessary, ensure that the dataset size is manageable and that the conversion is justified by specific requirements, such as integration with libraries that require Pandas DataFrames. |
There was a problem hiding this comment.
This sentence starts a bit weirdly. Maybe If the conversion... would work better?
| ---- | ||
| # Converting a PySpark DataFrame to a Pandas DataFrame | ||
| df = spark.read.csv("my_data.csv") | ||
| pandas_df = df.toPandas() # Noncompliant: May cause memory issues with large datasets |
There was a problem hiding this comment.
It would be nice to have both example to the same thing. Adding here the filtering on the Pandas DataFrame, I think would make sense.
|
|
You can preview this rule here (updated a few minutes after each push).
Review
A dedicated reviewer checked the rule description successfully for: