> On Mar 29, 2016, at 2:08 AM, Jeff Steinmetz <jeffrey.steinm...@gmail.com>
> wrote:
>
> I’m not affiliated to either author nor have any attachment to an specific
> outcome - and happy to continue being as objective and unbiased as possible.
>
>
> I would say they now have different philosophical approaches (as of the March
> 23rd merge of datalayer#7 to 702).
> I agree with Amos Elberg that 702 has changed directions a few times.
>
> Re: commits to 702 by Leemoonsoo on March 23 via datalayer#7:
> I found the current state of the 702 PR to be succinct, in terms of it’s
> code base, via its use of the SparkR dependency - which is already baked into
> spark distribution.
>
> The 702 code base appears to be easier to maintain using this approach (less
> code, no rscala source, no BSD licensing additions required, easier to read).
> 702 packages correctly with -Pbuild-distr as expected - i.e. it works out of
> gate from the distribution directory.
> The build profile -Psparkr worked as expected, and the addition of this
> profile felt intuitive to me.
>
>
> Myself and a colleague that uses R extensively noticed (as Amos Elberg
> reminded us):
> 208 handles passing arrays and other data types between scala & R more
> gracefully than 702.
> 208 handles the output of intermediate R calls more gracefully than 702.
>
> Beyond that:
> 208 Requires SPARK_HOME to be set or the interpreter hangs with no error.
> It’s been mentioned by the 208 author that the requirement to set SPARK_HOME
> is a feature. I think we could revisit this assumption now that I see how
> 702 handles this with defaults via a graceful fallback.
> 702 works fine with zero configuration, I.e for those that want to test
> locally with no separate spark distribution installed (SPARK_HOME does not
> need to be set).
> 702 having just an %r interpreter, and having it as part of the spark
> interpreter (same subdirectory) feels like a cleaner approach (this is
> arguably a philosophical difference again).
>
>
> It feels like an exhaustive list of `.z.show.googleVis(Motion)` type calls in
> 208 could bloom into unnecessary code maintenance overhead and required
> additions every time a new chart library is introduced, vs. a more generic
> show method. Perhaps a follow on improvement post merge (applies to both
> PRs).
> This same chart rendering works in 702 with `print(Motion, tag='chart’)`
> which isn’t necessarily better or worse, again, a different philosophical
> approach.
>
> They both have merit in different regards. It’s doesn’t feel appropriate to
> make a broad statement that "no-one supported 702”.
> If I had a magic wand, it would be a hybrid of the two approaches.
>
> I look forward to continuing the review of each PR individually or both
> collaboratively.
>
> Regards,
> Jeff
>
>
>> On 3/28/16, 4:13 PM, "Sourav Mazumder" <sourav.mazumde...@gmail.com> wrote:
>>
>> All said and done we had enough discussion on this point for many months
>> now. As far as I know, 208 is the PR which community/people have so far
>> used mostly and successfully (at least me and whoever I introduced to 208
>> for SparkR support). I thought it was going to be merged a long time ago.
>> May be what will make sense is to first integrate the 208. After that, a
>> new PR can be created on that and if 702 has anything extra then that
>> feature can be added.
>>
>> Regards,
>> Sourav
>>
>>
>>> On Mon, Mar 28, 2016 at 12:37 AM, Eran Witkon <eranwit...@gmail.com> wrote:
>>>
>>> @Elberg, If I were you I would ask myself why isn't the community taking
>>> part in this debate?
>>> Personally I prefer a team player as a contributor over the best developer.
>>> just my 2c
>>> Eran
>>>
>>>> On Mon, 28 Mar 2016 at 09:52 Amos B. Elberg <amos.elb...@gmail.com> wrote:
>>>>
>>>> Moon - I opened this discussion so it could take place with the community
>>>> as a whole, not just you.
>>>>
>>>> Suffice it to say, I disagree with every one of the technical claims
>>>> you've just made, and I don't trust your intent.
>>>>
>>>> Let the community process happen.
>>>>
>>>>> On Mar 28, 2016, at 2:47 AM, moon soo Lee <m...@apache.org> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> Simply put,
>>>>>
>>>>> - 702 and/or 208 will can merged as they're ready. [1]
>>>>> - 208 will not be merged while it does not pass CI. If you think code
>>> in
>>>>> 208 is not a problem but CI itself or other part of Zeppelin is
>>> problem,
>>>>> then that particular problem be fixed before merge 208.
>>>>> - 702 has proper integration test [2]
>>>>>
>>>>> I'm not sure why you're so hard at devaluating 702.
>>>>> 702 is not something you need to beat and win. 702 is something you
>>> need
>>>> to
>>>>> help / learn / collaborate.
>>>>>
>>>>> Will you able to show your ability to collaborate with other community
>>>>> members?
>>>>>
>>>>> Thanks,
>>>>> moon
>>>>>
>>>>> [1]
>>> http://apache-zeppelin-incubating-dev-mailing-list.75694.x6.nabble.com/R-interpreter-in-Zeppelin-further-steps-tp6967.html
>>>>> [2]
>>> https://github.com/apache/incubator-zeppelin/pull/702/files#diff-64a9440e811c5fba6ac1b61157fa6912R87
>>>>>
>>>>>
>>>>>> On Sun, Mar 27, 2016 at 7:11 PM Amos Elberg <amos.elb...@gmail.com>
>>>> wrote:
>>>>>>
>>>>>> I am saddened to have to start this thread *again*. While I thought
>>> we
>>>> had
>>>>>> reached consensus on this, several times over, apparently some people
>>>>>> disagree. I hope this will be the last time.
>>>>>>
>>>>>> With this thread, I am asking the community to reach consensus (1)
>>> That
>>>> 208
>>>>>> should be merged this week, without further delay; and (2) That Moon
>>> Lee
>>>>>> Soo and Felix Cheung take no further part in the discussions of 208
>>> and
>>>>>> 702.
>>>>>>
>>>>>> This PR has been pending since August. It has been stalled that entire
>>>> time
>>>>>> for no technical reason.
>>>>>>
>>>>>> We reached agreement to merge 208 in November, again in December, and
>>>> again
>>>>>> in February -- when Moon agreed to stay out of the issue. At that
>>>> point,
>>>>>> Alex, I, and others, began working on it, and appeared to be making
>>>>>> substantial progress.
>>>>>>
>>>>>> And then Alex just stopped. Instead, he commenced the thread saying
>>>> that a
>>>>>> consensus had to be reached on 208 and 702. Until that point,
>>>> essentially
>>>>>> no-one had paid attention to 702. In the discussion that followed, we
>>>>>> reached a consensus to merge 208 as soon as possible. After the
>>> thread
>>>> had
>>>>>> died, Alex asked if anyone had additional comments, and Moon popped-in
>>>> to
>>>>>> insist that both PRs be merged. Again, no-one supported 702. At all.
>>>>>>
>>>>>> Each time I said "we had a consensus before, does anyone want to
>>> change
>>>>>> it," Alex or Moon steered the discussion away. The final vote was not
>>>> to
>>>>>> merge 702 or merge "both" -- it was to treat them as normal PRs.
>>>> (Although
>>>>>> one person did want both merged simultaneously.) That would mean
>>>>>> completing 208 on its merits and then evaluating 702.
>>>>>>
>>>>>> At the time, I objected to the discussion, because I thought the whole
>>>>>> thing was a contrived excuse for Moon to reject 208 by pushing 702.
>>>> That
>>>>>> is exactly what he is now seeking to do.
>>>>>>
>>>>>> *Status of 208 & 702*
>>>>>>
>>>>>> PR 208 has been feature-complete and testable since early September.
>>> It
>>>>>> has been adopted by more than 1000 users, who I have been supporting
>>> for
>>>>>> more than six months. The code has not undergone any major changes
>>>> since
>>>>>> September. There are no known bugs, and no outstanding feature
>>> requests
>>>>>> that can be satisfied without major changes to the Zeppelin
>>>> architecture.
>>>>>>
>>>>>> 208 does *not* fail CI. 208 includes extensive unit tests of the
>>>> R-Spark
>>>>>> integration because this turned out to get broken by changes in
>>> Zeppelin
>>>>>> often. Because CI is unable at present to provide a consistent
>>>>>> environment, 208's *OWN UNIT TESTS*, which pass when run on an
>>> ordinary
>>>>>> machine, fail when run on CI.
>>>>>>
>>>>>> 208 does need a push for compatibility with a recently adopted PR --
>>>> that
>>>>>> is work I've essentially completed, but have not pushed.
>>>>>>
>>>>>> PR 702 is a re-design based on 208 -- not just architecture, but right
>>>> down
>>>>>> to the choice of demo images, which were taken from 208's
>>> documentation.
>>>>>> In fact, 702 has had been re-engineered several times to more closely
>>>>>> conform to 208's architecture and feature set. But 702 still remains
>>>>>> feature-incomplete -- it cannot handle the range of visualizations, R
>>>>>> classes, etc., that 208 can. It is not stable code, and shows no signs
>>>> of
>>>>>> stabilizing any time soon.
>>>>>>
>>>>>> No-one has adopted 702. It has changed radically, fundamentally, at
>>>> least
>>>>>> 4 times over the past two months since it was submitted. One of those
>>>>>> changes was only days ago.
>>>>>>
>>>>>> 702 also has no proper tests, which is the excuse for not merging 208.
>>>> 702
>>>>>> has things labelled "tests," but they don't actually attempt to
>>> connect
>>>> to
>>>>>> R or Spark, which are the things that break and which therefore need
>>>>>> testing.
>>>>>>
>>>>>> ***
>>>>>>
>>>>>> I would like credit for my own work and design. I think I have more
>>> than
>>>>>> earned that.
>
