Behavior of Top.Largest

[Wesley Tanaka <wt...@yahoo.com.INVALID>]

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: Behavior of Top.Largest

[Reuven Lax <re...@google.com.INVALID>]

Makes sense - however a bit sad that it then got reused by other unrelated
combiners (e.g. ApproximateQuantiles). Could we replace the internal uses
of this class with com.google.common.collect.Ordering.natural?

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.
>
> On Sun, May 14, 2017, 1:00 PM Reuven Lax <re...@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 <da...@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: Behavior of Top.Largest

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

On Mon, May 22, 2017 at 10:47 AM, Robert Bradshaw <
robertwb@google.com.invalid> wrote:
>
> For a simple rename like this, I would at least strongly encourage
> fixing both for those that are capable.
>

Agreed. My response suffered from premature generalization, wandering off
into "policy"-esque topics.

Kenn




> > 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.
> >>
> >> (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.)
> >>
> >> 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
> <re...@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: Behavior of Top.Largest

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

On Mon, May 22, 2017 at 10:24 AM, Kenneth Knowles
<kl...@google.com.invalid> wrote:
> There was a brief discussion on a JIRA about following the pattern like:
>
> Main task: API change proposal
>  - subtask: Java change
>  - subtask: Python change
>  - subtask: ... etc ...

We should at least do this.

> This allows asynchrony without losing track of our intent. Of course, I
> hope Beam is so successful that we have too many SDKs to keep them in sync
> :-)
>
> Aside from that, there are some of us who are as fluent in Python as in
> Java, but we shouldn't expect that to be universal. And we should always
> consider opinions of SDK authors as to what is most idiomatic for their
> language and SDK.

For a simple rename like this, I would at least strongly encourage
fixing both for those that are capable.

> 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.
>>
>> (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.)
>>
>> 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 <ro...@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 <re...@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 <da...@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: Behavior of Top.Largest

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

There was a brief discussion on a JIRA about following the pattern like:

Main task: API change proposal
 - subtask: Java change
 - subtask: Python change
 - subtask: ... etc ...

This allows asynchrony without losing track of our intent. Of course, I
hope Beam is so successful that we have too many SDKs to keep them in sync
:-)

Aside from that, there are some of us who are as fluent in Python as in
Java, but we shouldn't expect that to be universal. And we should always
consider opinions of SDK authors as to what is most idiomatic for their
language and SDK.

Kenn

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.
>
> (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.)
>
> 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 <ro...@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 <re...@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 <da...@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/
>> >> > >> > >
>> >> > >> >
>> >> > >
>> >> > >
>> >> >
>> >>
>>

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: Behavior of Top.Largest

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

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.

> 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 <ro...@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 <re...@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 <da...@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: Behavior of Top.Largest

[Dan Halperin <dh...@google.com.INVALID>]

I think this is an unrealistic request -- Python and Java workflows are
completely different, and Python developer documentation is especially
abysmal.

(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.)

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 <ro...@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 <re...@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 <da...@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: Behavior of Top.Largest

[Ahmet Altay <al...@google.com.INVALID>]

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 <ro...@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 <re...@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 <da...@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: Behavior of Top.Largest

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

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 <ro...@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 <re...@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 <da...@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: Behavior of Top.Largest

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

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 <re...@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 <da...@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: Behavior of Top.Largest

[Ben Chambers <bc...@google.com.INVALID>]

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.

On Sun, May 14, 2017, 1:00 PM Reuven Lax <re...@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 <da...@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: Behavior of Top.Largest

[Reuven Lax <re...@google.com.INVALID>]

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 <da...@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 <wt...@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: Behavior of Top.Largest

[Davor Bonaci <da...@apache.org>]

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 <wt...@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/