Hi,
> - Don't keep the old GropupStep at all, and have the new GroupStep "concat"
> the second and third 'by' traversals if they exist.
This is not possible. In fact, I messed up my original examples. They should be:
g.V.group.by('name').by(outE.weight).by(max(local)) // old way
g.V.group.by('name').by(outE.weight.max()) // new way
Notice the semantics of the reduce in the old way. Its looking for a
collection, not a stream. You can argue, well introspect and flatten/etc. That
is easy for max(), sum(), etc, but what about lambdas? What about complex
group().by()s people are using I don't even know about and mess up the
reasoning on?
> - Leave the GroupStep as-is, and add the mapValues step for whoever wants a
> better implementation:
mapValues() is already step in use.
http://tinkerpop.incubator.apache.org/docs/3.1.0-SNAPSHOT/#mapvalues-step
> g.V().group().by(keyTraversal).mapValues(valueTraversal).
> No deprecation needed.
Can't do it cause of mapValues() and we need to keep modulators consistent,
hence by()-modulation. I don't want to introduce a new modulator just for this
situation as it makes the language start to look hackish. "Oh mapValues() isn't
a step, its a modulator like by() as as() but only works for group() as the
second modulator?!" Right now we have as() and by() as our "special" modulating
steps.
Finally, to your notion of group(traversal). We have group(String) which uses
GroupSideEffectStep. Furthermore, all step(traversal) are used exclusively for
nesting and not for modulation (e.g. local(), repeat(), etc.). Using a
traversal parameter in this way would ruin the consistency of the language.
Thank you for your thoughts,
Marko.
http://markorodriguez.com
>
> On Wed, 7 Oct 2015 at 14:42 Ran Magen <[email protected]> wrote:
>
>> My point is that group().by('key').by('value/reduce') seems kind of weird
>> anyway. The equivalent sql would be "select 'value/reduce' ... group by
>> 'key' ".
>>
>> A few alternative options:
>> g.V().group().by(keyTraversal).mapValues(valueTraversal) //As Marko
>> pointed out this doesn't address the old GroupStep deprecation
>>
>> g.V().group(keyTraversal).mapValues(valueTraversal) // group() is
>> @Deprecated
>>
>> g.V().group(valueTraversal).by(keyTraversal) // group() is @Deprecated
>>
>> g.V().group(keyTraversal, valueTraversal) // group() is @Deprecated
>>
>>
>> On Wed, 7 Oct 2015 at 01:42 Marko Rodriguez <[email protected]> wrote:
>>
>>> Uh…. First select() is already taken and by() is always a
>>> by()-modulator…… :/ Even with that, we still can't fix the backwards
>>> compatibility issue as we can't deprecate out the second by(). How do you
>>> say "if the second by() is applied, its Deprecated?" Perhaps Logger.WARN or
>>> something.. Dunno, sorta odd.
>>>
>>> Marko.
>>>
>>> http://markorodriguez.com
>>>
>>> On Oct 6, 2015, at 3:26 PM, Ran Magen <[email protected]> wrote:
>>>
>>>> How about changing the second 'by' to a more indicative name (could
>>>> 'select' work?)?
>>>>
>>>> That way we could improve on the name (which always had me confused),
>>> and
>>>> leave the second+third 'by's as an indication to use the old
>>>> implementation...
>>>>
>>>> On יום ד׳, 7 באוק׳ 2015 at 0:18 Marko Rodriguez <[email protected]>
>>>> wrote:
>>>>
>>>>> Hello everyone (and the NSA -- Kuppitz did it),
>>>>>
>>>>> This is regarding this ticket:
>>>>> https://issues.apache.org/jira/browse/TINKERPOP3-866
>>>>>
>>>>> In group_step_2/ branch we have a new implementation of GroupStep.
>>> Here is
>>>>> an example:
>>>>>
>>>>> g.V.group.by('name').by(outE.weight).by(max()) // old way
>>>>> g.V.group.by('name').by(outE.weight.max()) // new way
>>>>>
>>>>> What is the difference, there is only two by()-modulators that can be
>>>>> passed to group(): keyTraversal and valueTraversal. What happened to
>>>>> reduceTraversal? Well, valueTraversal is that. If you want to reduce on
>>>>> your values, well, add a reducing barrier step -- e.g. max, min, sum,
>>>>> count, fold, mean, etc. In the old way, we would put all your values
>>> into a
>>>>> Collection<V> and then when the step was complete, we would feed those
>>> into
>>>>> the reduceTraversal. Now -- we can dynamically reduce on the fly. This
>>> will
>>>>> greatly improve speed and memory usage as we don't have to create a
>>> big old
>>>>> Collection<V> and instead, can reduce as new traversers are fed into
>>>>> valueTraversal.
>>>>>
>>>>> So this is great and no would object. What sucks is that this is NOT
>>>>> backwards compatible. What Stephen and I came up with (in the ticket)
>>> is to
>>>>> make:
>>>>>
>>>>>
>>>>>
>>> https://github.com/apache/incubator-tinkerpop/blob/group_step_2/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java#L637-L639
>>>>>
>>>>> This new step groupV3d0() is a deprecated step that is what the "old
>>> way"
>>>>> was. Note the naming convention that Stephen used for Kryo is now used
>>> here
>>>>> -- V3d0 (version 3<dot>0). Thus, for users moving forward, they will
>>> have
>>>>> to do a "replace all" on their queries and rename "group()" to
>>>>> "groupV3d0()". Then slowly, over time, convert their traversal to use
>>> the
>>>>> new group() step and its respective semantics.
>>>>>
>>>>> Note that in the past we have been able to do backwards compatibility
>>> via
>>>>> having the new step (e.g. AddVertexStep) simulate the behavior of the
>>> old
>>>>> step and have respective @Deprecates in there to say that particular
>>>>> GraphTraversal methods will go away. However, in this situation, the
>>> best
>>>>> we can do is groupV3d0().
>>>>>
>>>>> Anywho. That is that. If people have any better ideas on how to solve
>>> this
>>>>> backwards compatibility issue, I'm all (deaf) ears.
>>>>>
>>>>> Thanks,
>>>>> Marko.
>>>>>
>>>>> http://markorodriguez.com
>>>>>
>>>>>
>>>
>>>
