You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/05/31 06:23:02 UTC
[GitHub] [spark] LuciferYang opened a new pull request, #36732: [SPARK-39345][CORE][SQL][DSTREAM][MLLIB] Replace `filter(!condition)` with `filterNot(condition)`
LuciferYang opened a new pull request, #36732:
URL: https://github.com/apache/spark/pull/36732
### What changes were proposed in this pull request?
This pr replace `filter(!condition)` with `filterNot(condition)` .
### Why are the changes needed?
Use appropriate api.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Pass Github Actions
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #36732: [SPARK-39345][CORE][SQL][DSTREAM][ML][MESOS][SS] Replace `filter(!condition)` with `filterNot(condition)`
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #36732:
URL: https://github.com/apache/spark/pull/36732#issuecomment-1142188003
cc @wangyum
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #36732: [SPARK-39345][CORE][SQL][DSTREAM][ML][MESOS][SS] Replace `filter(!condition)` with `filterNot(condition)`
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on code in PR #36732:
URL: https://github.com/apache/spark/pull/36732#discussion_r885245947
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusListener.scala:
##########
@@ -413,7 +413,7 @@ class SQLAppStatusListener(
if (other != exec) other.stages else Nil
}.toSet
stageMetrics.keySet().asScala
- .filter(!activeStages.contains(_))
+ .diff(activeStages)
Review Comment:
`.filter(!activeStages.contains(_)) `
->
`.filterNot(activeStages.contains)`
->
`.diff(activeStages)`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #36732: [SPARK-39345][CORE][SQL][DSTREAM][ML][MESOS][SS] Replace `filter(!condition)` with `filterNot(condition)`
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #36732:
URL: https://github.com/apache/spark/pull/36732#issuecomment-1148250861
> No, we wouldn't backport this, that's more change. Does this offer any benefit? I'm not sure it's more readable even.
If the readability is not improved, let me close this pr.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on pull request #36732: [SPARK-39345][CORE][SQL][DSTREAM][ML][MESOS][SS] Replace `filter(!condition)` with `filterNot(condition)`
Posted by GitBox <gi...@apache.org>.
wangyum commented on PR #36732:
URL: https://github.com/apache/spark/pull/36732#issuecomment-1146723234
cc @srowen
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #36732: [SPARK-39345][CORE][SQL][DSTREAM][ML][MESOS][SS] Replace `filter(!condition)` with `filterNot(condition)`
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #36732:
URL: https://github.com/apache/spark/pull/36732#issuecomment-1146991307
> The change is OK; I don't think it adds any performance. The only hesitation here would be code churn and possible merge conflicts due to this. I'm kind of neutral on it, I think it's OK.
If this acceptable, I can give pr for 3.3, 3.2 and 3.1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on pull request #36732: [SPARK-39345][CORE][SQL][DSTREAM][ML][MESOS][SS] Replace `filter(!condition)` with `filterNot(condition)`
Posted by GitBox <gi...@apache.org>.
srowen commented on PR #36732:
URL: https://github.com/apache/spark/pull/36732#issuecomment-1147422759
No, we wouldn't backport this, that's more change.
Does this offer any benefit? I'm not sure it's more readable even.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang closed pull request #36732: [SPARK-39345][CORE][SQL][DSTREAM][ML][MESOS][SS] Replace `filter(!condition)` with `filterNot(condition)`
Posted by GitBox <gi...@apache.org>.
LuciferYang closed pull request #36732: [SPARK-39345][CORE][SQL][DSTREAM][ML][MESOS][SS] Replace `filter(!condition)` with `filterNot(condition)`
URL: https://github.com/apache/spark/pull/36732
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org