Return-Path: X-Original-To: apmail-flink-dev-archive@www.apache.org Delivered-To: apmail-flink-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id C3D4019C2C for ; Thu, 28 Apr 2016 12:02:35 +0000 (UTC) Received: (qmail 43724 invoked by uid 500); 28 Apr 2016 12:02:35 -0000 Delivered-To: apmail-flink-dev-archive@flink.apache.org Received: (qmail 43656 invoked by uid 500); 28 Apr 2016 12:02:35 -0000 Mailing-List: contact dev-help@flink.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@flink.apache.org Delivered-To: mailing list dev@flink.apache.org Received: (qmail 43645 invoked by uid 99); 28 Apr 2016 12:02:35 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd4-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 28 Apr 2016 12:02:35 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd4-us-west.apache.org (ASF Mail Server at spamd4-us-west.apache.org) with ESMTP id F318EC06CB for ; Thu, 28 Apr 2016 12:02:34 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd4-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 1 X-Spam-Level: * X-Spam-Status: No, score=1 tagged_above=-999 required=6.31 tests=[KAM_LAZY_DOMAIN_SECURITY=1, RCVD_IN_DNSWL_NONE=-0.0001] autolearn=disabled Received: from mx1-lw-eu.apache.org ([10.40.0.8]) by localhost (spamd4-us-west.apache.org [10.40.0.11]) (amavisd-new, port 10024) with ESMTP id zJPp-DL6Zz-t for ; Thu, 28 Apr 2016 12:02:32 +0000 (UTC) Received: from mx1.mailbox.org (mx1.mailbox.org [80.241.60.212]) by mx1-lw-eu.apache.org (ASF Mail Server at mx1-lw-eu.apache.org) with ESMTPS id 5032D5F298 for ; Thu, 28 Apr 2016 12:02:32 +0000 (UTC) Received: from smtp1.mailbox.org (smtp1.mailbox.org [80.241.60.240]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.mailbox.org (Postfix) with ESMTPS id 4F44642691 for ; Thu, 28 Apr 2016 14:02:26 +0200 (CEST) X-Virus-Scanned: amavisd-new at heinlein-support.de Received: from smtp1.mailbox.org ([80.241.60.240]) by hefe.heinlein-support.de (hefe.heinlein-support.de [91.198.250.172]) (amavisd-new, port 10030) with ESMTP id 3Lfsjdl1vtcy for ; Thu, 28 Apr 2016 14:02:24 +0200 (CEST) Subject: Re: Eclipse Problems To: dev@flink.apache.org References: <571D454B.9000408@apache.org> <571E0197.6010207@apache.org> <571FC47C.9090605@apache.org> <5720A103.4080808@apache.org> <5720CBCD.80909@apache.org> From: "Matthias J. Sax" Message-ID: <5721FB56.6@apache.org> Date: Thu, 28 Apr 2016 14:00:22 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="elFoiDcrfC2LUgWfpoVGMGxSGGXiH8iN3" --elFoiDcrfC2LUgWfpoVGMGxSGGXiH8iN3 Content-Type: multipart/mixed; boundary="E0fvplAPgSi8xBl44GXcIxJgx9OgPnBdi" From: "Matthias J. Sax" To: dev@flink.apache.org Message-ID: <5721FB56.6@apache.org> Subject: Re: Eclipse Problems References: <571D454B.9000408@apache.org> <571E0197.6010207@apache.org> <571FC47C.9090605@apache.org> <5720A103.4080808@apache.org> <5720CBCD.80909@apache.org> In-Reply-To: --E0fvplAPgSi8xBl44GXcIxJgx9OgPnBdi Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Maybe. As we cannot change the interface anyway and is does what the user expects (even if only "by accident") I would just leave it as is... And for "mixed" types, specifying the common base class is acceptable "overhead" for the user IMHO. Nevertheless, the Eclipse problem is still not solved. Any ideas about th= is? If not, I would just leave this problem open, too. It is "only" for two test classes and not severe. -Matthias On 04/28/2016 10:13 AM, Till Rohrmann wrote: > I don't know whether it is that difficult to find out the nearest commo= n > super type. At least the Java compiler does the same when calling the p= ure > var arg method. I think, we should be able to do something similar. Als= o > throwing out the pure var arg implementation is not ideal in my opinion= , > because this would force the user to always specify class objects even > though he's providing only a set of strings, for example. >=20 > On Wed, Apr 27, 2016 at 4:25 PM, Matthias J. Sax wro= te: >=20 >> I guess, removing .fromElements(Object..) would fix the problem. Not >> sure so, if we can remove the method due to API stability... >> >> I don't see any other good solution (even if the current implementatio= n >> gives a nice behavior by accident...): >> >> If you have a complex class hierarchy, it would be quite complex to fi= nd >> out the correct common sub-type. Using only .fromElemenst(Class, >> X...) requires to specify the correct sub-type and has the additional >> advantage, the the compiler can check the type already (instead of a >> potential later runtime error). >> >> >> -Matthias >> >> >> On 04/27/2016 03:07 PM, Till Rohrmann wrote: >>> You=E2=80=99re completely right Mathias. The compiler shouldn=E2=80=99= t allow something >>> like env.fromElements(SubClass.class, new ParentClass()) if it weren=E2= =80=99t >> for >>> the overloaded method. Thus, the test case is somewhat bogus. >>> >>> I=E2=80=99m actually wondering why the initial problem >>> https://issues.apache.org/jira/browse/FLINK-3444 was solved this way.= I >>> think it would be better to automatically infer the common super type= of >>> all provided elements. Otherwise, you run into problems you=E2=80=99v= e found out >>> about. >>> >>> Consequently, I think it is fine if you remove the >> fromElementsWithBaseType2 >>> test case. >>> >>> Cheers, >>> Till >>> =E2=80=8B >>> >>> On Wed, Apr 27, 2016 at 1:22 PM, Matthias J. Sax >> wrote: >>> >>>> Hi Till, >>>> >>>> but StreamExecutionEnvironmentTest.fromElementWithBaseTypeTest2 does= not >>>> test was you describe -- even if it is intended to test it. >>>> >>>> It would test your describe scenario, if fromElements(Class, X...= ) >>>> would be called, But this call is not possible because X is defined = a >>>> type Subclass and thus the provided object of Parentclass cannot be >>>> handed over as type X. Therefore, fromElements(Object...) is called:= of >>>> course, this fails too, because now the type is derived as >>>> Class (and not Subclass) and neither Subclass nor Parentcl= ass >>>> inherit from Class. >>>> >>>> The scenario you describe will never work -- if you remove the overl= oad >>>> fromElements(Object...) the code would not even compile as the compi= ler >>>> can figure out from the generics that the call >>>> fromElments(Subclass.class, new Parentclass()) is invalid. >>>> >>>> It is only possible to hand in "reverse inheritance types" for >>>> fromElemenst(Object...). In this case, the first given Object define= s >>>> the type. Thus, if you call fromElements(new Subclass(), new >>>> Parentclass()), the call will fail, as Parentclass is no subtype of >>>> Subtype -- the call fromElements(new Parentclass() new Subclass()) w= ould >>>> succeed. >>>> >>>> Makes sense? >>>> >>>> Still no idea how to make it compile in Eclipse... >>>> >>>> -Matthias >>>> >>>> On 04/27/2016 10:21 AM, Till Rohrmann wrote: >>>>> Thanks for looking into this problem Mathias. I think the Scala tes= t >>>> should >>>>> be fixed as you've proposed. >>>>> >>>>> Concerning the >>>> StreamExecutionEnvironmentTest.fromElementWithBaseTypeTest2, >>>>> I think it shouldn't be changed. The reason is that the class defin= es >> the >>>>> common base class of the elements. And the test makes sure that the= >>>>> fromElements call fails if you provide instances which are not of t= he >>>>> specified type or a subclass of it. Thus, we should find another wa= y to >>>>> make it work with Eclipse. >>>>> >>>>> Cheers, >>>>> Till >>>>> >>>>> On Tue, Apr 26, 2016 at 9:41 PM, Matthias J. Sax = >>>> wrote: >>>>> >>>>>> Even if the fix works, I still have two issues in my Eclipse build= =2E.. >>>>>> >>>>>> In >>>>>> >>>>>> >>>>>> >>>> >> flink-scala/src/test/scala/org/apache/flink/api/scala/extensions/base/= AcceptPFTestBase.scala >>>>>> >>>>>> Eclipse cannot infer the integer type. It could be fixed if you ma= ke >> the >>>>>> type explicit (as this is only a test, it might be nice to fix thi= s -- >>>>>> let me know if I can push this or not) >>>>>> >>>>>>> diff --git >>>>>> >>>> >> a/flink-scala/src/test/scala/org/apache/flink/api/scala/extensions/bas= e/AcceptPFTestBase.scala >>>>>> >>>> >> b/flink-scala/src/test/scala/org/apache/flink/api/scala/extensions/bas= e/AcceptPFTestBase.scala >>>>>>> index c2e13fe..f9ce3b8 100644 >>>>>>> --- >>>>>> >>>> >> a/flink-scala/src/test/scala/org/apache/flink/api/scala/extensions/bas= e/AcceptPFTestBase.scala >>>>>>> +++ >>>>>> >>>> >> b/flink-scala/src/test/scala/org/apache/flink/api/scala/extensions/bas= e/AcceptPFTestBase.scala >>>>>>> @@ -29,7 +29,7 @@ private[extensions] abstract class AcceptPFTest= Base >>>>>> extends TestLogger with JUni >>>>>>> >>>>>>> private val env =3D ExecutionEnvironment.getExecutionEnvironme= nt >>>>>>> >>>>>>> - protected val tuples =3D env.fromElements(1 -> "hello", 2 -> >> "world") >>>>>>> + protected val tuples =3D env.fromElements(new Integer(1) -> "h= ello", >>>>>> new Integer(2) -> "world") >>>>>>> protected val caseObjects =3D env.fromElements(KeyValuePair(1,= >>>>>> "hello"), KeyValuePair(2, "world")) >>>>>>> >>>>>>> protected val groupedTuples =3D tuples.groupBy(_._1) >>>>>> >>>>>> Furthermore, in >>>>>> >>>>>> >>>> >> flink-java/src/test/java/org/apache/flink/api/java/io/FromElementsTest= =2Ejava >>>>>> >>>>>>> @Test >>>>>>> public void fromElementsWithBaseTypeTest1() { >>>>>>> ExecutionEnvironment executionEnvironment =3D >>>>>> ExecutionEnvironment.getExecutionEnvironment(); >>>>>>> executionEnvironment.fromElements(ParentType.class, new >>>> SubType(1, >>>>>> "Java"), new ParentType(1, "hello")); >>>>>>> } >>>>>> >>>>>> and in >>>>>> >>>>>> >>>>>> >>>> >> flink-streaming-java/src/test/java/org/apache/flink/streaming/api/Stre= amExecutionEnvironmentTest.java >>>>>> >>>>>>> @Test >>>>>>> public void fromElementsWithBaseTypeTest1() { >>>>>>> StreamExecutionEnvironment env =3D >>>>>> StreamExecutionEnvironment.getExecutionEnvironment(); >>>>>>> env.fromElements(ParentClass.class, new SubClass(1, "Java")= , >> new >>>>>> ParentClass(1, "hello")); >>>>>>> } >>>>>> >>>>>> In both cases, I get the error: >>>>>> >>>>>> The method .fromElements(Object[]) is ambiguous >>>>>> >>>>>> No clue how to fix this, and why Eclipse does not bind to >>>>>> .fromElements(Class, X). Any ideas? >>>>>> >>>>>> I also digger a little bit and for both test-classes there is a se= cond >>>>>> test method called "fromElementsWithBaseTypeTest2". If I understan= d >> this >>>>>> test correctly, it also tries to bind to .fromElements(Class, X= ), >> but >>>>>> this does not happen and .fromElemenst(Object[]) is called. Even i= f >>>>>> there is still an exception, I got the impression that this test d= oes >>>>>> not what the intention was. >>>>>> >>>>>> If might be good to change fromElementsWithBaseTypeTest2 to >>>>>> >>>>>>> env.fromElements(new SubClass(1, "Java"), new ParentClass(1, >> "hello")); >>>>>> >>>>>> (ie, remove the first Class parameter). Any comments on this? >>>>>> >>>>>> -Matthias >>>>>> >>>>>> >>>>>> >>>>>> On 04/25/2016 01:42 PM, Robert Metzger wrote: >>>>>>> Cool, thank you for working on this! >>>>>>> >>>>>>> On Mon, Apr 25, 2016 at 1:37 PM, Matthias J. Sax >>>>>> wrote: >>>>>>> >>>>>>>> I can confirm that the SO answer works. >>>>>>>> >>>>>>>> I will add a note to the Eclipse setup guide at the web site. >>>>>>>> >>>>>>>> -Matthias >>>>>>>> >>>>>>>> >>>>>>>> On 04/25/2016 11:33 AM, Robert Metzger wrote: >>>>>>>>> It seems that the user resolved the issue on SO, right? >>>>>>>>> >>>>>>>>> On Mon, Apr 25, 2016 at 11:31 AM, Ufuk Celebi >>>> wrote: >>>>>>>>> >>>>>>>>>> On Mon, Apr 25, 2016 at 12:14 AM, Matthias J. Sax < >> mjsax@apache.org >>>>> >>>>>>>>>> wrote: >>>>>>>>>>> What do you think about this? >>>>>>>>>> >>>>>>>>>> Hey Matthias! >>>>>>>>>> >>>>>>>>>> Thanks for bringing this up. >>>>>>>>>> >>>>>>>>>> I think it is very desirable to keep support for Eclipse. It's= >>>> quite a >>>>>>>>>> high barrier for new contributors to enforce a specific IDE >>>> (although >>>>>>>>>> IntelliJ is gaining quite the user base I think :P). >>>>>>>>>> >>>>>>>>>> Do you have time to look into this? >>>>>>>>>> >>>>>>>>>> =E2=80=93 Ufuk >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>> >>>> >>>> >>> >> >> >=20 --E0fvplAPgSi8xBl44GXcIxJgx9OgPnBdi-- --elFoiDcrfC2LUgWfpoVGMGxSGGXiH8iN3 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJXIftbAAoJEFCVK48prEZ4+gsP/0bJCWqMcxhZ0DvnGwiqbIeq WW+WdK4fzepzlaqB5TT4W10RcZ105HXLgx6I+6BbSQxa2xUo4085FDrzmuTUcxg4 aZKd9TZcQGhDfaNSude9RoBOcRoNpL7q1/m14Gym0kiJRNTb7pET+2LkC6kooXNP 4jA/lN5v1rqUuHIdt71OnEnNuxt4044F77ectaQgIrceR4eeGr/QLVIjbgBGC0JD StnRyqE/BYVKflqV5m7KSogBv7WO7KkgdfBEWH1Hpsa5TPrRZ9CenK+k2+KqT7Ps uYCyD5/WybsQSN7v3NI1iJDumZoU29pyBjRm/OG/on7oRrtX54CUe3P70KboF0NR wSBMldv19yk0cn5/zPeXxy+XbgDnlWMqz6s/g+tfAdrnPpVaAfWIJAbReKIkvUsK lR1skzDYatdr8v00SC4QqDEvOD0VOzFFSv4L+MxgLYw/gJBQ73x4uGEMXJwG7cZs Yeau8aNVV/KEdcXqjoCzMH+Ut7Q3UJYlv0LtffSWqbhdg4PZtTSM4dK95nPK88CI rjKboYCOa1h/UgDjdp+cWi450YCRU3Dtoor1FfTJBha4f2wECDqTR1Tiicl0ZUEE MejrMvCX+HB8Jw50f1Li4f/hFUY/fMS2w3oj+3+Or+IHln//usMoQoMcdrFrkwhw xnyx5CMXfjUgLVbp3goY =C0nZ -----END PGP SIGNATURE----- --elFoiDcrfC2LUgWfpoVGMGxSGGXiH8iN3--