-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-26640][CORE][ML][SQL][STREAMING][PYSPARK] Code cleanup from lgtm.com analysis #23571
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
Conversation
|
|
||
| var endPoint = createRESTEndPointForExecutorsPage(appId); | ||
| $.getJSON(endPoint, function (response, status, jqXHR) { | ||
| var summary = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused variables
| { | ||
| data: function (row, type) { | ||
| return type === 'display' ? (formatDuration(row.allTotalDuration, type) + ' (' + formatDuration(row.allTotalGCTime, type) + ')') : row.allTotalDuration | ||
| return type === 'display' ? (formatDuration(row.allTotalDuration) + ' (' + formatDuration(row.allTotalGCTime) + ')') : row.allTotalDuration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused JS function args
| historySummary = $("#history-summary"); | ||
| searchString = historySummary["context"]["location"]["search"]; | ||
| requestedIncomplete = getParameterByName("showIncomplete", searchString); | ||
| var historySummary = $("#history-summary"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implicit var -> explicit
| "hasMultipleAttempts": hasMultipleAttempts, | ||
| "showCompletedColumns": !requestedIncomplete, | ||
| } | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implicit semicolon -> explicit
| from __future__ import print_function | ||
|
|
||
| from pyspark.sql import SparkSession | ||
| from pyspark.ml.param import Params |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused imports
| precision = metrics.precision() | ||
| recall = metrics.recall() | ||
| f1Score = metrics.fMeasure() | ||
| precision = metrics.precision(1.0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed this when removing deprecated no-arg method
| conf.put(SparkLauncher.DRIVER_EXTRA_CLASSPATH, value); | ||
| break; | ||
| case CONF: | ||
| checkArgument(value != null, "Missing argument to %s", CONF); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Flags that this seems like it wants to check for null as it's check elsewhere
| sock = None | ||
| else: | ||
| raise Exception("could not open socket: %s" % errors) | ||
| raise Exception("could not open socket: %s" % errors) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
else: with for doesn't do anything different if there's no break
| :return: model instance | ||
| """ | ||
| raise NotImplemented | ||
| raise NotImplementedError |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NotImplemented is not actually an error that can be thrown
| '"field6":[{"field7": "row2"}]}', | ||
| '{"field1" : null, "field2": "row3", ' | ||
| '"field3":{"field4":33, "field5": []}}' | ||
| '{"field1" : 2, "field3":{"field4":22, "field5": [10, 11]},"field6":[{"field7": "row2"}]}', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confusing string concatenation across lines
|
Test build #101336 has finished for PR 23571 at commit
|
|
Test build #101337 has finished for PR 23571 at commit
|
Use `numpy.array` instead of `array` in doctests (PR 23571)
|
Test build #101368 has finished for PR 23571 at commit
|
|
Merged to master |
…tm.com analysis ## What changes were proposed in this pull request? Misc code cleanup from lgtm.com analysis. See comments below for details. ## How was this patch tested? Existing tests. Closes apache#23571 from srowen/SPARK-26640. Lead-authored-by: Sean Owen <[email protected]> Co-authored-by: Hyukjin Kwon <[email protected]> Co-authored-by: Sean Owen <[email protected]> Signed-off-by: Sean Owen <[email protected]>
What changes were proposed in this pull request?
Misc code cleanup from lgtm.com analysis. See comments below for details.
How was this patch tested?
Existing tests.