[MINOR][BUILD] ensure call to translate_component has correct number of arguments

[ghost]

What changes were proposed in this pull request?

The call to translate_component only supplied 2 out of the 3 required arguments. I added a default empty list for the missing argument to avoid a run-time error.

I work for Semmle, and noticed the bug with our LGTM code analyzer:
https://lgtm.com/projects/g/apache/spark/snapshot/0655f1624ff7b73e5c8937ab9e83453a5a3a4466/files/dev/create-release/releaseutils.py?sort=name&dir=ASC&mode=heatmap#x1434915b6576fb40:1

How was this patch tested?

I checked that ./dev/run-tests pass OK.

[srowen]

Heh, I'm not even sure this is called. This doesn't do anything with the warnings, so, you could also just remove the unused warnings arg.

[srowen]

@ipwright BTW I love Semmle / LGTM and have had a look through the warnings... I can open a PR to fix a lot of the simple ones. It uncovered at least one non-trivial bug: Pyspark vector classes don't actually implement unary negation correctly. It throws an error. Fixing that ...

[SparkQA]

Test build #4520 has finished for PR 23567 at commit bac22f3.

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

[srowen]

Merged to master

[ghost]

@srowen Thanks for merging, and very glad to hear you like LGTM.

I'm happy to submit PRs that fix the simple alerts, if that will save you time (please advise). I helped develop LGTM's code quality algorithms, and so I'm interested in knowing how much fixing and clean-up effort is required to move a project up a grade. E.g., Spark is doing good:



However, the top possible rating is A+. And so I want to get a handle on the effort required to get the top grade.

I recommend enabling LGTM automated code review on Apache Spark, since it detects problems prior to merging.
https://lgtm.com/help/lgtm/managing-pr-integration#enabling-pr-integration

Best wishes,
Ian.

[srowen]

Thanks @ipwright , I opened #23571 to fix many of the rest of the small issues, and #23570 to fix a bug. I'm a fan of static analysis just because it occasionally catches real bugs.

The only minor issue I find is that the 'unused import' detection doesn't account for doctest usages.

The Java code was already pretty OK as IntelliJ inspection had turned up most of the problems, but I don't think we'd made a comprehensive effort on Python.

Oege mentioned that Scala analysis might come eventually, which would obviously be really good too, but is a lot harder.

[ghost]

Thanks @srowen, and thanks for fixing those issues.

If you have the time, could you point me to an example where unused import detection fails to account for doctest uses? Once I have an example, I can ask our Python team to refine the query.

Thanks!

[srowen]

@ipwright as an example, in https://lgtm.com/projects/g/apache/spark/snapshot/eabef9f5294181257dc2d5bc39e77652101f48ac/files/python/pyspark/mllib/clustering.py?sort=name&dir=ASC&mode=heatmap#xfc95d54cf2a4e9cd:1 , DenseVector is reported as unused but is referenced in the doctest. Maybe there is something more subtle going on here but it seems like the import is needed

[ghost]

@srowen Thanks for the example, which is super helpful. Our Python team has a fix in mind, and we're on the case!

P.S. I forgot to mention that you can activate LGTM automated code review from this page:
https://lgtm.com/projects/g/apache/spark/ci/

[srowen]

I think we can't enable it as we don't have admin access but can check on the code status periodically