flink-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthias J. Sax" <mj...@apache.org>
Subject Re: Eclipse Problems
Date Wed, 27 Apr 2016 14:25:17 GMT
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 <mjsax@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 <mjsax@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 <mjsax@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 <uce@apache.org>
>> 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?
>>>>>>>>
>>>>>>>> – Ufuk
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
> 


Mime
View raw message