Matthias, Won't this be a compile-time error as long as the user is parameterizing the return type since .fromElements(OUT...) returns DataStreamSource<OUT> and will bind to the nearest common superclass? The new .fromElements(Class<OUT>, OUT...) does give the user the choice of common superclass.
Greg On Wed, Apr 27, 2016 at 10:25 AM, Matthias J. Sax <mj...@apache.org> wrote: > 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 implementation > gives a nice behavior by accident...): > > If you have a complex class hierarchy, it would be quite complex to find > out the correct common sub-type. Using only .fromElemenst(Class<X>, > 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’re completely right Mathias. The compiler shouldn’t allow something > > like env.fromElements(SubClass.class, new ParentClass()) if it weren’t > for > > the overloaded method. Thus, the test case is somewhat bogus. > > > > I’m 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’ve found out > > about. > > > > Consequently, I think it is fine if you remove the > fromElementsWithBaseType2 > > test case. > > > > Cheers, > > Till > > > > > > On Wed, Apr 27, 2016 at 1:22 PM, Matthias J. Sax <mj...@apache.org> > 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>, 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<Subclass> (and not Subclass) and neither Subclass nor Parentclass > >> inherit from Class<Subclass>. > >> > >> The scenario you describe will never work -- if you remove the overload > >> fromElements(Object...) the code would not even compile as the compiler > >> 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 defines > >> 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()) would > >> 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 test > >> should > >>> be fixed as you've proposed. > >>> > >>> Concerning the > >> StreamExecutionEnvironmentTest.fromElementWithBaseTypeTest2, > >>> I think it shouldn't be changed. The reason is that the class defines > 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 the > >>> specified type or a subclass of it. Thus, we should find another way to > >>> make it work with Eclipse. > >>> > >>> Cheers, > >>> Till > >>> > >>> On Tue, Apr 26, 2016 at 9:41 PM, Matthias J. Sax <mj...@apache.org> > >> wrote: > >>> > >>>> Even if the fix works, I still have two issues in my Eclipse build... > >>>> > >>>> 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 make > the > >>>> type explicit (as this is only a test, it might be nice to fix this -- > >>>> let me know if I can push this or not) > >>>> > >>>>> diff --git > >>>> > >> > a/flink-scala/src/test/scala/org/apache/flink/api/scala/extensions/base/AcceptPFTestBase.scala > >>>> > >> > b/flink-scala/src/test/scala/org/apache/flink/api/scala/extensions/base/AcceptPFTestBase.scala > >>>>> index c2e13fe..f9ce3b8 100644 > >>>>> --- > >>>> > >> > a/flink-scala/src/test/scala/org/apache/flink/api/scala/extensions/base/AcceptPFTestBase.scala > >>>>> +++ > >>>> > >> > b/flink-scala/src/test/scala/org/apache/flink/api/scala/extensions/base/AcceptPFTestBase.scala > >>>>> @@ -29,7 +29,7 @@ private[extensions] abstract class AcceptPFTestBase > >>>> extends TestLogger with JUni > >>>>> > >>>>> private val env = ExecutionEnvironment.getExecutionEnvironment > >>>>> > >>>>> - protected val tuples = env.fromElements(1 -> "hello", 2 -> > "world") > >>>>> + protected val tuples = env.fromElements(new Integer(1) -> "hello", > >>>> new Integer(2) -> "world") > >>>>> protected val caseObjects = env.fromElements(KeyValuePair(1, > >>>> "hello"), KeyValuePair(2, "world")) > >>>>> > >>>>> protected val groupedTuples = tuples.groupBy(_._1) > >>>> > >>>> Furthermore, in > >>>> > >>>> > >> > flink-java/src/test/java/org/apache/flink/api/java/io/FromElementsTest.java > >>>> > >>>>> @Test > >>>>> public void fromElementsWithBaseTypeTest1() { > >>>>> ExecutionEnvironment executionEnvironment = > >>>> 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/StreamExecutionEnvironmentTest.java > >>>> > >>>>> @Test > >>>>> public void fromElementsWithBaseTypeTest1() { > >>>>> StreamExecutionEnvironment env = > >>>> 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>, X). Any ideas? > >>>> > >>>> I also digger a little bit and for both test-classes there is a second > >>>> test method called "fromElementsWithBaseTypeTest2". If I understand > this > >>>> test correctly, it also tries to bind to .fromElements(Class<X>, X), > but > >>>> this does not happen and .fromElemenst(Object[]) is called. Even if > >>>> there is still an exception, I got the impression that this test does > >>>> 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 <mj...@apache.org> > >>>> 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 <u...@apache.org> > >> wrote: > >>>>>>> > >>>>>>>> On Mon, Apr 25, 2016 at 12:14 AM, Matthias J. Sax < > mj...@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? > >>>>>>>> > >>>>>>>> – Ufuk > >>>>>>>> > >>>>>>> > >>>>>> > >>>>>> > >>>>> > >>>> > >>>> > >>> > >> > >> > > > >
