? … I would know you were using the "old way" by the fact that you added a 3rd by()-modulator. I don't see how multi-properties effects this particular situation you bring up. ?
Marko. http://markorodriguez.com On Oct 7, 2015, at 12:12 PM, Daniel Kuppitz <[email protected]> wrote: >> >> g.V.group.by('name').by(outE.weight).by(max(local)) >> If you did this, I would know you were using "the old way" as your >> provided a reduce by()-modulator. > > > Actually you wouldn't know. Think about multi-properties and: > > g.V().group().by('name').by(values('weight')).by(max(local)) > > > Cheers, > Daniel > > > On Wed, Oct 7, 2015 at 7:57 PM, Marko Rodriguez <[email protected]> > wrote: > >> Hi everyone, >> >> I was soooooo close to having group() be backwards compatible. However, I >> found a corner case that makes it break. >> >> If you do something like: >> >> g.V.group.by('name').by(outE.weight).by(max(local)) >> >> If you did this, I would know you were using "the old way" as your >> provided a reduce by()-modulator. I would simply make your valueTraversal >> be "outE.weight.fold." Easy peasy. In fact, all the test cases passed. But >> that doesn't mean its right as the tests don't cover every situation. >> >> What about if you don't add a reduceTraversal and you want "the old way?" >> >> gremlin> g.V.group.by('name').by(outE.weight) // old way >> ==>[marko:[0.4,0.5,0.1]] >> >> gremlin> g.V.group.by('name').by(outE.weight) // new way >> ==>[marko:0.4] >> >> As a side ---- note that if you do this in the new way, I assume a fold() >> and thus, its just like the old way. Makes sense as people really mean >> name.fold() and you don't want people to have to type values("name").fold(). >> >> gremlin> g.V.group.by(label).by(name) // new way >> ==>[software:[lop,ripple]] >> ==>[person:[marko,josh,vadas,peter]] >> >> So, back to the outE.weight example ----- I was thinking, well we can say >> that if the valueTraversal doesn't end with a reducing barrier step, then >> we add fold() and tada, the old and new are the same. However, what about >> this situation: >> >> g.V.group.by('name').by(outE.weight.max.mult(2)) >> >> Its not so simple…. now the end step is not a reducing barrier. >> >> Thus, if the user doesn't provide a reduceTraversal (the 3rd >> by()-modulation), then we don't know which version of GroupStep they are >> interested in. >> >> Blows, >> Marko. >> >> http://markorodriguez.com >> >> On Oct 7, 2015, at 7:28 AM, Marko Rodriguez <[email protected]> wrote: >> >>> 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 >>>>>>>> >>>>>>>> >>>>>> >>>>>> >>> >> >>
