Proper developer instructions for Python SDK

[Chamikara Jayalath <ch...@apache.org>]

(moving to a separate thread)

On Mon, May 22, 2017 at 10:45 AM Robert Bradshaw
<ro...@google.com.invalid> wrote:

> On Sun, May 21, 2017 at 11:45 AM, Dan Halperin
> <dh...@google.com.invalid> wrote:
> > I think this is an unrealistic request -- Python and Java workflows are
> > completely different, and Python developer documentation is especially
> > abysmal.
>
> We should improve this for sure.
>
> > (E.g., I had to have Robert sit with me to get the Python SDK to work at
> > all on my developer machine, and even then I gave up and chmod-ed my
> > machine-wide Python repos to be world-writable to get it to work.)
>
> FWIW, this is only because of trying to use our Maven pom files
> (which, among other things, are virtualenv-incompatible) to work on
> Python.
>


I believe one issue here is contribution guide being Java SDK/Maven
focussed. Created https://issues.apache.org/jira/browse/BEAM-2340.

Thanks,
Cham


>
> > On Fri, May 19, 2017 at 4:50 PM, Ahmet Altay <al...@google.com.invalid>
> > wrote:
> >
> >> I mentioned this in the PR, I believe it is worth repeating here.
> >>
> >> It is important to keep the API consistent between Python and Java. It
> >> would help a lot, if changes are applied to both SDKs at the same time.
> If
> >> that is not possible, an easier alternative would be to file a JIRA
> issue
> >> so that the work could be tracked in the other SDK.
> >>
> >> Ahmet
> >>
> >> On Fri, May 19, 2017 at 4:22 PM, Robert Bradshaw <
> >> robertwb@google.com.invalid> wrote:
> >>
> >> > I see this was implemented. Do we have a policy/guideline for when a
> >> > name is "bad enough" to merit renaming (and keeping a duplicate,
> >> > deprecated member around for a year or more).
> >> >
> >> > On Mon, May 15, 2017 at 9:25 AM, Robert Bradshaw <robertwb@google.com
> >
> >> > wrote:
> >> > > On Sun, May 14, 2017 at 3:36 PM, Ben Chambers
> >> > <bc...@google.com.invalid>
> >> > > wrote:
> >> > >>
> >> > >> Exposing the CombineFn is necessary for use with composed combine
> or
> >> > >> combining value state. There may be other reasons we made this
> >> visible,
> >> > >> but
> >> > >> these continue to justify it.
> >> > >
> >> > >
> >> > > These are the CompareFns, not the CombineFns.
> >> > >
> >> > > It'd be nicer to use the Guava and/or Java8 natural ordering
> >> comparables,
> >> > > but they don't promise serializable.
> >> > >
> >> > > I agree the naming is unfortunate, but I don't know that it's bad
> >> enough
> >> > to
> >> > > introduce a new name and have duplication and deprecation in the
> API.
> >> It
> >> > > also goes deeper than this; Top.of(...) gives elements in
> *decreasing*
> >> > order
> >> > > while List.sort(...) gives elements in *increasing* order so using a
> >> > > comparator in one will always produce the opposite effect of using a
> >> > > comparator in the other.
> >> > >
> >> > >>
> >> > >> On Sun, May 14, 2017, 1:00 PM Reuven Lax <relax@google.com.invalid
> >
> >> > wrote:
> >> > >>
> >> > >> > I believe the reason why this is called Top.largest, is that
> >> > originally
> >> > >> > it
> >> > >> > was simply the comparator used by Top.largest - i.e. and
> >> > implementation
> >> > >> > detail. At some point it was made public and used by other
> >> transforms
> >> > -
> >> > >> > maybe making an implementation detail a public class was the real
> >> > >> > mistake?
> >> > >> >
> >> > >> > On Sun, May 14, 2017 at 11:45 AM, Davor Bonaci <davor@apache.org
> >
> >> > wrote:
> >> > >> >
> >> > >> > > I agree this is an unfortunate name.
> >> > >> > >
> >> > >> > > Tangential: can we rename APIs now that the first stable
> release
> >> is
> >> > >> > nearly
> >> > >> > > done?
> >> > >> > > Of course -- the "rename" can be done by introducing a new API,
> >> and
> >> > >> > > deprecating, but not removing, the old one. Then, once we
> decide
> >> to
> >> > >> > > move
> >> > >> > to
> >> > >> > > the next major release, the deprecated API can be removed.
> >> > >> > >
> >> > >> > > I think we should probably do the "rename" at some point, but
> I'd
> >> > >> > > leave
> >> > >> > the
> >> > >> > > final call to the wider consensus.
> >> > >> > >
> >> > >> > > On Sat, May 13, 2017 at 5:16 PM, Wesley Tanaka
> >> > >> > > <wtanaka@yahoo.com.invalid
> >> > >> > >
> >> > >> > > wrote:
> >> > >> > >
> >> > >> > > > Using Top.Largest to sort a list of {2,1,3} produces {1,2,3}.
> >> > This
> >> > >> > > > matches the javadoc for the class, but seems
> counter-intuitive
> >> --
> >> > >> > > > one
> >> > >> > > might
> >> > >> > > > expect that a Comparator called Largest would give largest
> items
> >> > >> > > > first.
> >> > >> > > > I'm wondering if renaming the classes to Natural / Reversed
> >> would
> >> > >> > better
> >> > >> > > > match their behavior?
> >> > >> > > >
> >> > >> > > > ---
> >> > >> > > > Wesley Tanaka
> >> > >> > > > https://wtanaka.com/
> >> > >> > >
> >> > >> >
> >> > >
> >> > >
> >> >
> >>
>

Re: Proper developer instructions for Python SDK

[Kenneth Knowles <kl...@google.com.INVALID>]

On Mon, May 22, 2017 at 11:21 AM, Robert Bradshaw <
robertwb@google.com.invalid> wrote:

> In addition, I also would like to see the realization of the
> (previously suggested) "mvn tips/tricks/handy commands."


+1000

I'd contribute if someone else starts it off. I've got a few. Seeing what
folks are having to do to get the job done might inspire new configuration
xml.

Kenn

Re: Proper developer instructions for Python SDK

[Robert Bradshaw <ro...@google.com.INVALID>]

On Mon, May 22, 2017 at 11:13 AM, Chamikara Jayalath
<ch...@apache.org> wrote:
> (moving to a separate thread)
>
> On Mon, May 22, 2017 at 10:45 AM Robert Bradshaw
> <ro...@google.com.invalid> wrote:
>
>> On Sun, May 21, 2017 at 11:45 AM, Dan Halperin
>> <dh...@google.com.invalid> wrote:
>> > I think this is an unrealistic request -- Python and Java workflows are
>> > completely different, and Python developer documentation is especially
>> > abysmal.
>>
>> We should improve this for sure.
>>
>> > (E.g., I had to have Robert sit with me to get the Python SDK to work at
>> > all on my developer machine, and even then I gave up and chmod-ed my
>> > machine-wide Python repos to be world-writable to get it to work.)
>>
>> FWIW, this is only because of trying to use our Maven pom files
>> (which, among other things, are virtualenv-incompatible) to work on
>> Python.
>>
>
> I believe one issue here is contribution guide being Java SDK/Maven
> focussed. Created https://issues.apache.org/jira/browse/BEAM-2340.

Thanks. I also created https://issues.apache.org/jira/browse/BEAM-2341 .

In addition, I also would like to see the realization of the
(previously suggested) "mvn tips/tricks/handy commands." I've wasted
way too many hours trying to figure out how to properly build/test the
Java code, especially when it comes to cross-project dependencies and
changes (though hopefully I've bugged enough Java folks to be out of
the woods there).

>> > On Fri, May 19, 2017 at 4:50 PM, Ahmet Altay <al...@google.com.invalid>
>> > wrote:
>> >
>> >> I mentioned this in the PR, I believe it is worth repeating here.
>> >>
>> >> It is important to keep the API consistent between Python and Java. It
>> >> would help a lot, if changes are applied to both SDKs at the same time.
>> If
>> >> that is not possible, an easier alternative would be to file a JIRA
>> issue
>> >> so that the work could be tracked in the other SDK.
>> >>
>> >> Ahmet
>> >>
>> >> On Fri, May 19, 2017 at 4:22 PM, Robert Bradshaw <
>> >> robertwb@google.com.invalid> wrote:
>> >>
>> >> > I see this was implemented. Do we have a policy/guideline for when a
>> >> > name is "bad enough" to merit renaming (and keeping a duplicate,
>> >> > deprecated member around for a year or more).
>> >> >
>> >> > On Mon, May 15, 2017 at 9:25 AM, Robert Bradshaw <robertwb@google.com
>> >
>> >> > wrote:
>> >> > > On Sun, May 14, 2017 at 3:36 PM, Ben Chambers
>> >> > <bc...@google.com.invalid>
>> >> > > wrote:
>> >> > >>
>> >> > >> Exposing the CombineFn is necessary for use with composed combine
>> or
>> >> > >> combining value state. There may be other reasons we made this
>> >> visible,
>> >> > >> but
>> >> > >> these continue to justify it.
>> >> > >
>> >> > >
>> >> > > These are the CompareFns, not the CombineFns.
>> >> > >
>> >> > > It'd be nicer to use the Guava and/or Java8 natural ordering
>> >> comparables,
>> >> > > but they don't promise serializable.
>> >> > >
>> >> > > I agree the naming is unfortunate, but I don't know that it's bad
>> >> enough
>> >> > to
>> >> > > introduce a new name and have duplication and deprecation in the
>> API.
>> >> It
>> >> > > also goes deeper than this; Top.of(...) gives elements in
>> *decreasing*
>> >> > order
>> >> > > while List.sort(...) gives elements in *increasing* order so using a
>> >> > > comparator in one will always produce the opposite effect of using a
>> >> > > comparator in the other.
>> >> > >
>> >> > >>
>> >> > >> On Sun, May 14, 2017, 1:00 PM Reuven Lax <relax@google.com.invalid
>> >
>> >> > wrote:
>> >> > >>
>> >> > >> > I believe the reason why this is called Top.largest, is that
>> >> > originally
>> >> > >> > it
>> >> > >> > was simply the comparator used by Top.largest - i.e. and
>> >> > implementation
>> >> > >> > detail. At some point it was made public and used by other
>> >> transforms
>> >> > -
>> >> > >> > maybe making an implementation detail a public class was the real
>> >> > >> > mistake?
>> >> > >> >
>> >> > >> > On Sun, May 14, 2017 at 11:45 AM, Davor Bonaci <davor@apache.org
>> >
>> >> > wrote:
>> >> > >> >
>> >> > >> > > I agree this is an unfortunate name.
>> >> > >> > >
>> >> > >> > > Tangential: can we rename APIs now that the first stable
>> release
>> >> is
>> >> > >> > nearly
>> >> > >> > > done?
>> >> > >> > > Of course -- the "rename" can be done by introducing a new API,
>> >> and
>> >> > >> > > deprecating, but not removing, the old one. Then, once we
>> decide
>> >> to
>> >> > >> > > move
>> >> > >> > to
>> >> > >> > > the next major release, the deprecated API can be removed.
>> >> > >> > >
>> >> > >> > > I think we should probably do the "rename" at some point, but
>> I'd
>> >> > >> > > leave
>> >> > >> > the
>> >> > >> > > final call to the wider consensus.
>> >> > >> > >
>> >> > >> > > On Sat, May 13, 2017 at 5:16 PM, Wesley Tanaka
>> >> > >> > > <wtanaka@yahoo.com.invalid
>> >> > >> > >
>> >> > >> > > wrote:
>> >> > >> > >
>> >> > >> > > > Using Top.Largest to sort a list of {2,1,3} produces {1,2,3}.
>> >> > This
>> >> > >> > > > matches the javadoc for the class, but seems
>> counter-intuitive
>> >> --
>> >> > >> > > > one
>> >> > >> > > might
>> >> > >> > > > expect that a Comparator called Largest would give largest
>> items
>> >> > >> > > > first.
>> >> > >> > > > I'm wondering if renaming the classes to Natural / Reversed
>> >> would
>> >> > >> > better
>> >> > >> > > > match their behavior?
>> >> > >> > > >
>> >> > >> > > > ---
>> >> > >> > > > Wesley Tanaka
>> >> > >> > > > https://wtanaka.com/
>> >> > >> > >
>> >> > >> >
>> >> > >
>> >> > >
>> >> >
>> >>
>>