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/12/28 00:38:06 UTC
[GitHub] [spark] tedyu opened a new pull request, #39250: [SQL][MINOR] Use Diamond operator for constructing HashMap
tedyu opened a new pull request, #39250:
URL: https://github.com/apache/spark/pull/39250
### What changes were proposed in this pull request?
This PR uses Diamond operator for constructing HashMap for type inference.
### Why are the changes needed?
The change follows Java practices for creating new HashMap.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Existing test suite.
--
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 closed pull request #39250: [SQL][MINOR] Use Diamond operator for constructing HashMap
Posted by GitBox <gi...@apache.org>.
srowen closed pull request #39250: [SQL][MINOR] Use Diamond operator for constructing HashMap
URL: https://github.com/apache/spark/pull/39250
--
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] tedyu commented on pull request #39250: [SQL][MINOR] Use Diamond operator for constructing HashMap
Posted by GitBox <gi...@apache.org>.
tedyu commented on PR #39250:
URL: https://github.com/apache/spark/pull/39250#issuecomment-1366305206
@srowen
I have covered `JavaBeanDeserializationSuite`
--
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] tedyu commented on pull request #39250: [SQL][MINOR] Use Diamond operator for constructing HashMap
Posted by GitBox <gi...@apache.org>.
tedyu commented on PR #39250:
URL: https://github.com/apache/spark/pull/39250#issuecomment-1367136917
@srowen
Please let me know what else should be done for 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] LuciferYang commented on pull request #39250: [SQL][MINOR] Use Diamond operator for constructing HashMap
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #39250:
URL: https://github.com/apache/spark/pull/39250#issuecomment-1366660173
We can use IJ inspections `Java | Java language level migration aids | Java 7 | Explicit type can be replaced with '<>'` to find all
--
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 #39250: [SQL][MINOR] Use Diamond operator for constructing HashMap
Posted by GitBox <gi...@apache.org>.
srowen commented on PR #39250:
URL: https://github.com/apache/spark/pull/39250#issuecomment-1366299627
OK yeah I ran IJ inspections too. That's the only collection. There is one other related one I think we could fix, in JavaBeanDeserializationSuite :
```
List<Tuple2<String, Item>> expectedRecords = Arrays.asList(
new Tuple2("a", new Item("a", 8)),
new Tuple2("b", new Item("b", 3)),
new Tuple2("c", new Item("c", 2)));
```
Tuple2 should have empty angle brackets.
--
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 #39250: [SQL][MINOR] Use Diamond operator for constructing HashMap
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on code in PR #39250:
URL: https://github.com/apache/spark/pull/39250#discussion_r1058073382
##########
sql/core/src/test/java/test/org/apache/spark/sql/JavaBeanDeserializationSuite.java:
##########
@@ -590,9 +590,9 @@ public Item call(Item item1, Item item2) throws Exception {
.reduceGroups(rf);
List<Tuple2<String, Item>> expectedRecords = Arrays.asList(
- new Tuple2("a", new Item("a", 8)),
- new Tuple2("b", new Item("b", 3)),
- new Tuple2("c", new Item("c", 2)));
+ new Tuple2<>("a", new Item("a", 8)),
Review Comment:
There should be many similar cases in the test code. Because they are test codes, I have not touched them before to avoid conflicts. If we need to fix them, I think we should fix all in one.
WDYT @HyukjinKwon
--
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 #39250: [SQL][MINOR] Use Diamond operator for constructing HashMap
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #39250:
URL: https://github.com/apache/spark/pull/39250#issuecomment-1366426696
for example
https://github.com/apache/spark/blob/87a235c2143449bd8da0acee4ec3cd99993155bb/sql/core/src/test/java/test/org/apache/spark/sql/JavaDatasetSuite.java#L168
can use `new Tuple2<>(2, 2L)` instead of `new Tuple2<Integer, Long>(2, 2L)`, do you think this is a similar case?
--
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 #39250: [SQL][MINOR] Use Diamond operator for constructing HashMap
Posted by GitBox <gi...@apache.org>.
srowen commented on PR #39250:
URL: https://github.com/apache/spark/pull/39250#issuecomment-1367348662
Was just waiting for any more feedback. Merged to master
--
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] tedyu commented on pull request #39250: [SQL][MINOR] Use Diamond operator for constructing HashMap
Posted by GitBox <gi...@apache.org>.
tedyu commented on PR #39250:
URL: https://github.com/apache/spark/pull/39250#issuecomment-1366289684
I searched for `LinkedList`, `HashSet`, `ArrayList` and `HashMap` among the java files.
This is the only one I found.
--
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 #39250: [SQL][MINOR] Use Diamond operator for constructing HashMap
Posted by GitBox <gi...@apache.org>.
srowen commented on PR #39250:
URL: https://github.com/apache/spark/pull/39250#issuecomment-1366684112
Yes, though that catches a lot of cases like specifying the generic type explicitly, where "<>" is sufficient. We don't need to 'fix' those, really. They aren't even warnings.
--
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 #39250: [SQL][MINOR] Use Diamond operator for constructing HashMap
Posted by GitBox <gi...@apache.org>.
srowen commented on PR #39250:
URL: https://github.com/apache/spark/pull/39250#issuecomment-1366658285
I think that still generates a compiler warning though, and that's the point of this change. You have to say 'use the inferred generic types'. This PR is about the "<>" syntax in constructors, not all instances of generic types used without generic types specified. There might be a few more, but this is all I saw too.
--
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 #39250: [SQL][MINOR] Use Diamond operator for constructing HashMap
Posted by GitBox <gi...@apache.org>.
srowen commented on PR #39250:
URL: https://github.com/apache/spark/pull/39250#issuecomment-1366289099
Are there more instances? please fix all at once, if there are more
--
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] tedyu commented on pull request #39250: [SQL][MINOR] Use Diamond operator for constructing HashMap
Posted by GitBox <gi...@apache.org>.
tedyu commented on PR #39250:
URL: https://github.com/apache/spark/pull/39250#issuecomment-1366547484
The types have been specified for `Tuple2` on line 168 in JavaDatasetSuite.java .
I think no change is needed there.
--
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] tedyu commented on pull request #39250: [SQL][MINOR] Use Diamond operator for constructing HashMap
Posted by GitBox <gi...@apache.org>.
tedyu commented on PR #39250:
URL: https://github.com/apache/spark/pull/39250#issuecomment-1366283040
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