Skip to content
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

Pyspark Linting Rules #7272

Open
6 tasks
sbrugman opened this issue Sep 11, 2023 · 5 comments
Open
6 tasks

Pyspark Linting Rules #7272

sbrugman opened this issue Sep 11, 2023 · 5 comments
Labels
plugin Implementing a known but unsupported plugin

Comments

@sbrugman
Copy link
Contributor

sbrugman commented Sep 11, 2023

Apache Spark is widely used in the python ecosystem for distributed computing. As user of spark I would like for ruff to lint problematic behaviours. The automation that ruff offers is especially useful in projects with various levels of software engineering skills, e.g. where people has more of a statistics background.

There exists a pyspark style guide and pylint extension.

I would like to start contributing a rule that checks for repeated use of withColumn:

This method introduces a projection internally. Therefore, calling it multiple times, for instance, via loops in order to add multiple columns can generate big plans which can cause performance issues and even StackOverflowException. To avoid this, use select() with multiple columns at once.

This violation seems common in existing code bases.
Are you ok with a PR introducing "Spark-specific rules" (e.g. SPK)?

  • SPK001: repeated withColumn usage, use withColumns or select
  • SPK002: repeated withColumnRenamed usage, use withColumnsRenamed
  • SPK003: repeated drop usage, consolidate in single call
  • SPK004: F.date_format with simple argument, replace with specialised function (e.g. F.hour)
  • SPK005: direct access column selection (e.g. F.lower(df.col)), use implicit column selection (e.g. F.lower(F.col("col")))
  • SPK006: unnecessary F.col (in F.lower(F.col('my_column'))), use F.lower('my_column').

ruff includes rules that are specific to third party libraries: numpy, pandas and airflow. Spark support would be a nice addition.

I would like to close with the following thought: supporting third-party packages may at first seem to be effort in the long tail of possible rules to add to ruff. Why not focus only on rules that affect all Python users? I hope that adding these will lead to creating helper functions that make adding new rules easier. I also think that these libraries will end up with similar API design patterns, that can be linted across the ecosystem. As an example, call chaining is common for many packages that perform transformations.

@charliermarsh
Copy link
Member

charliermarsh commented Sep 12, 2023

I'm generally open to adding package-specific rule sets for extremely popular packages (as with Pandas, NumPy, etc.), and Spark would fit that description. However, it'd be nice to have a few rules lined up before we move forward and add any one of them. Otherwise, we run the risk that we end up with really sparse categories that only contain a rule or two.

@charliermarsh charliermarsh added the plugin Implementing a known but unsupported plugin label Sep 12, 2023
@sbrugman
Copy link
Contributor Author

sbrugman commented Sep 12, 2023

Super. I've updated the issue with a couple of rules that we can track. I'll kick off with SPK001-3

@guilhem-dvr
Copy link

guilhem-dvr commented Feb 9, 2024

Hi, I was looking for such a thread.

To add to the proposed list, here are some rules we wish we had at my company:

  • unnecessary drop followed by a select
  • use unionByName instead of union / unionAll
  • use df.writeTo(...).append() instead of df.write.insertInto(...)
  • use df.writeTo(...).overwritePartitions() instead of df.write.insertInto(..., overwrite=True)
  • replace udf with native spark functions
  • alias pyspark.sql.functions to F -> from pyspark.sql import ..., functions as F, ...

@amadeuspzs
Copy link

Just to add that I would be interested in this functionality.

Also, the first link in the original post is broken, and the pylint extension looks unmaintained?

@stkrzysiak
Copy link

Super. I've updated the issue with a couple of rules that we can track. I'll kick off with SPK001-3

Did you get going with this? Thinking about jumping on it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin Implementing a known but unsupported plugin
Projects
None yet
Development

No branches or pull requests

5 participants