[GitHub] tinkerpop issue #759: TINKERPOP-1846 Fixed bug in LambdaRestrictionStrategy
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/759 VOTE +1 ---
[GitHub] tinkerpop issue #755: TINKERPOP-1834: Consider iterate() as a first class st...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/755 The bug is in GremlinServer's `TraversalOpProcessor`. I have it fixed. Running integration tests now. ---
[GitHub] tinkerpop pull request #755: TINKERPOP-1834: Consider iterate() as a first c...
GitHub user okram opened a pull request: https://github.com/apache/tinkerpop/pull/755 TINKERPOP-1834: Consider iterate() as a first class step https://issues.apache.org/jira/browse/TINKERPOP-1834 This is a re-issuing of a previously closed commit. In this model, a `NoneStep` was added as well as a `Traversal.none()`. `Traversal.iterate()` calls `Traversal.none()` if the traversal has not been fully compiled. The benefit of this is that `iterate()` will add a full filter and thus, for remote systems (non-embedded), all data is "iterated" server-side before being sent back to the client. This is a novel play in that `iterate()`, while being a terminal step, is now effecting the bytecode representation. cc/ @BrynCooke VOTE +1 You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1834 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/755.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #755 commit 2a957bb586263b2dd70b4db36d1b3c6f87d5596f Author: Marko A. Rodriguez Date: 2017-11-21T11:44:32Z Added NoneStep which simply filter(false). Traversal.none() was added which appends the NoneStep. Traversal.iterate() was updated such that if the traversal has not been compiled yet, then Traversal.none() is called to ensure that a full filter is propagated in the bytecode. ---
[GitHub] tinkerpop issue #748: TINKERPOP-1834: Consider iterate() as a first class st...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/748 Closing. Going to provide a simpler solution. ---
[GitHub] tinkerpop pull request #748: TINKERPOP-1834: Consider iterate() as a first c...
Github user okram closed the pull request at: https://github.com/apache/tinkerpop/pull/748 ---
[GitHub] tinkerpop issue #748: TINKERPOP-1834: Consider iterate() as a first class st...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/748 A `cap()` step is a `SupplyBarrierStep` and thus, requires an emission -- a single emission, but an emission nonetheless. The ultimate solution step has to a `FilterStep` by nature so it can truly return "nothing." However, we are now getting into particulars that don't need arguing. The concept is what matters. Now that I get the problem, you simply don't want to have to "reattach" and send data back if you don't have to, then the solution is, filter everything as the last step. That is, have `iterate()` append to the bytecode a "filter all." Easy. How that is done, just needs some fiddlin' around. ---
[GitHub] tinkerpop issue #748: TINKERPOP-1834: Consider iterate() as a first class st...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/748 Yes, `filter(false)` is just "the concept." The step would be: ``` filter(FalseTraversal.instance()) ``` Or as @dkuppitz says -- `not(identity())`. ---
[GitHub] tinkerpop issue #748: TINKERPOP-1834: Consider iterate() as a first class st...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/748 There is nothing for "the server" to know. Thus, gutting the introspection. By appending `filter(false)` to the bytecode, you have a traversal that returns nothing. Which is exactly what you want. No need for the receiving/executing engine to reason about anything. A 3-line change to `iterate()` would do the trick. ---
[GitHub] tinkerpop issue #748: TINKERPOP-1834: Consider iterate() as a first class st...
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/748
I think we over engineered this ticket. I believe @dkuppitz has the best
idea.
```
public Traversal iterate() {
this.filter(false);
while(hasNext()) {
next();
}
return this;
}
```
---
[GitHub] tinkerpop issue #748: TINKERPOP-1834: Consider iterate() as a first class st...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/748 Makes sense. ---
[GitHub] tinkerpop issue #748: TINKERPOP-1834: Consider iterate() as a first class st...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/748 ``` [INFO] [INFO] BUILD SUCCESS [INFO] [INFO] Total time: 02:30 h [INFO] Finished at: 2017-11-15T12:13:52-07:00 [INFO] Final Memory: 164M/1526M [INFO] ``` ---
[GitHub] tinkerpop pull request #748: TINKERPOP-1834: Consider iterate() as a first c...
GitHub user okram opened a pull request: https://github.com/apache/tinkerpop/pull/748 TINKERPOP-1834: Consider iterate() as a first class step https://issues.apache.org/jira/browse/TINKERPOP-1834 When a user triggers a "terminal method" (e.g. `toList()`, `toSet()`, `iterate()`), that method is now appended to the traversal's `Bytecode`. This allows providers to know what the user's intention is for the results of the traversal. This can allow them to make internal optimizations about how to execute the traversal. The drawback of this is that `Translators` need to know about which steps are "terminal" and to avoid applying them during translation. To make this easy, `Translator.TERMINAL_STEPS` is a `public static final` field to allow easy introspection. VOTE +1. You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1834 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/748.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #748 commit 3adfe7ec66d8cf24f3e8090ffe8dd557600b5b6c Author: Marko A. Rodriguez Date: 2017-11-15T16:11:26Z Added terminal method to the Traversal bytecode so providers know what the user used to trigger the evaluation. ---
[GitHub] tinkerpop issue #746: TINKERPOP-1829 Made detachment configurable in EventSt...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/746 VOTE +1 ---
[GitHub] tinkerpop issue #744: TINKERPOP-1802: hasId() fails for empty collections
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/744 ``` [INFO] [INFO] BUILD SUCCESS [INFO] [INFO] Total time: 04:37 h [INFO] Finished at: 2017-11-06T21:46:16-07:00 [INFO] Final Memory: 162M/1713M [INFO] ``` ---
[GitHub] tinkerpop pull request #744: TINKERPOP-1802: hasId() fails for empty collect...
GitHub user okram opened a pull request: https://github.com/apache/tinkerpop/pull/744 TINKERPOP-1802: hasId() fails for empty collections https://issues.apache.org/jira/browse/TINKERPOP-1802 If `hasId([])` is specified, then an `ArrayOutOfBoundsException` occurs. This has been fixed by simply filtering out (`filter(true)` in essence) all vertices once a `hasId([])` is reached. cc/ @rjbriody VOTE +1. You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1802 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/744.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #744 commit 74ca03dea1a7db7b2af39f46020cf8a75a2ea5c4 Author: Marko A. Rodriguez Date: 2017-11-06T21:36:22Z fixed a hasId([]) ArrayOutOfBoundsException bug that occurs in the rare situation where a user provides an empty collection of ids. Test cases developed by @dkuppitz. ---
[GitHub] tinkerpop pull request #743: TINKERPOP-1813: Subgraph step requires the grap...
GitHub user okram opened a pull request: https://github.com/apache/tinkerpop/pull/743 TINKERPOP-1813: Subgraph step requires the graph API https://issues.apache.org/jira/browse/TINKERPOP-1813 This ticket wanted `SubgraphStep` to not use the structure API. This is not something we will support in TinkerPop3. The structure API is a fundamental layer to our step API. It was advised to the provider to simply write their own custom `ProviderSubgraphStep` which avoids the structure API as they see fit. However, what was accomplished in this ticket was to change `SubgraphTest` to only use traversals (process API) in its assertions. This theme is good -- our process test suite should never rely on the structure API (beyond `ReferenceXXX` data). VOTE +1 You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1813 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/743.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #743 commit 797937eb0c0b0fa3bc177925f4e584dfd53d8f13 Author: Marko A. Rodriguez Date: 2017-11-02T16:07:10Z changed the SubgraphTest to use process API in assertions instead of structure API. This is in line with the current refactor trend to make sure the process API (and test suite) does not leak into the structure API. We need clear boundaries between the two APIs. ---
[GitHub] tinkerpop pull request #740: TINKERPOP-1814: Some process tests require the ...
GitHub user okram opened a pull request: https://github.com/apache/tinkerpop/pull/740 TINKERPOP-1814: Some process tests require the graph API https://issues.apache.org/jira/browse/TINKERPOP-1814 Fixed assertions in the test cases outlined by @BrynCooke that use the structure API when they should stick with the process API. I found more areas than specified in the ticket. I also suspect that other process test cases will have similar problems. As these are exposed, please just make a new ticket accordingly - besides being monotonously boring, these are easy to fix. VOTE +1. You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1814 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/740.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #740 commit 608f786d354fe419a30d1453452aa9f34308f0b4 Author: Marko A. Rodriguez Date: 2017-11-01T16:19:16Z fixed up various ProcessTestSuite tests that were using Strucure API in assertions. Converted to be traversals for analysis. ---
[GitHub] tinkerpop issue #739: TINKERPOP-1821: Consistent behavior of self-referencin...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/739 ``` [INFO] [INFO] BUILD SUCCESS [INFO] [INFO] Total time: 04:38 h [INFO] Finished at: 2017-10-30T20:11:57-06:00 [INFO] Final Memory: 170M/1547M [INFO] ``` ---
[GitHub] tinkerpop pull request #739: TINKERPOP-1821: Consistent behavior of self-ref...
GitHub user okram opened a pull request: https://github.com/apache/tinkerpop/pull/739 TINKERPOP-1821: Consistent behavior of self-referencing edges https://issues.apache.org/jira/browse/TINKERPOP-1821 Added test for self-edges and fixed a semantic issue in `Neo4jGraph` regarding self edges not being repeated on `BOTH`. VOTE +1. You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1821 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/739.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #739 commit 7f640f7e6d863cde1858d6bfa3ff502fd93a8663 Author: Stephen Mallette Date: 2017-10-30T15:06:02Z TINKERPOP-1821 Added tests for consistent traversal behavior around self-referencing edges commit 5d68ca17160e4977eddfdb688f93d37440897957 Author: Marko A. Rodriguez Date: 2017-10-30T21:28:37Z fixed up a self-edge test and Neo4jVertex to support repeat edges on BOTH. ---
[GitHub] tinkerpop issue #734: TINKERPOP-1801: fix profile() timing in OLAP by adding...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/734 VOTE +1 ---
[GitHub] tinkerpop issue #735: TINKERPOP-1803: inject() doesn't re-attach with remote...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/735 ``` [INFO] [INFO] BUILD SUCCESS [INFO] [INFO] Total time: 03:21 h [INFO] Finished at: 2017-10-18T21:13:52-06:00 [INFO] Final Memory: 163M/1660M [INFO] ``` ---
[GitHub] tinkerpop pull request #735: TINKERPOP-1803: inject() doesn't re-attach with...
GitHub user okram opened a pull request: https://github.com/apache/tinkerpop/pull/735 TINKERPOP-1803: inject() doesn't re-attach with remote traversals https://issues.apache.org/jira/browse/TINKERPOP-1803 Fixed an "attachement"-bug in `InjectStep` with a solution generalized to `StartStep`. VOTE +1 You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1803 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/735.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #735 commit 79b621c9a0ddc2d96f951c54ee3f1db3c8490d4c Author: Marko A. Rodriguez Date: 2017-10-18T22:45:16Z Fixed an attachement-bug in with a solution generalized to . ---
[GitHub] tinkerpop issue #734: TINKERPOP-1801: fix profile() timing in OLAP by adding...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/734 This is a nice update @artem-aliev because it doesn't change API and it is general for all `GraphComputer` implementations. Great! A couple things please for a solid VOTE. 1. Please update the `CHANGELOG.asciidoc` with the change you made. 2. In this PR discussion, please provide a `CUT/PASTE` of what the new metrics `toString()` looks like so people can judge its merits. Thank you. ---
[GitHub] tinkerpop issue #721: TINKERPOP-1786 Recipe and missing manifest items for S...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/721 VOTE +1 ---
[GitHub] tinkerpop issue #731: TINKERPOP-1650: PathRetractionStrategy makes Match ste...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/731 VOTE +1 ---
[GitHub] tinkerpop pull request #730: TINKERPOP-1797: LambdaRestrictionStrategy and L...
GitHub user okram opened a pull request: https://github.com/apache/tinkerpop/pull/730 TINKERPOP-1797: LambdaRestrictionStrategy and LambdaMapStep in by()-modulation. https://issues.apache.org/jira/browse/TINKERPOP-1797 We have had too many problems with `LambdaRestrictionStrategy` because it is difficult to know what is a true lambda. I have now simply hardcoded the lambda determination as a `String` analysis of the lambda object for Java, Groovy, and Python. As we add more GLVs, we can add more string mappers. Every other solution thus far has either been too lenient or too restrictive. VOTE +1 You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1797 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/730.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #730 commit 46ac055f747390eefb58ac7ab7ef4c9fc178aaec Author: Marko A. Rodriguez Date: 2017-10-09T17:20:26Z We have had so many problems with LambdaRestrictionStragegy because it is difficult to know what is a true lambda. I have now simply hardcoded the lambda determination as a String analysis of the lambda object for Java, Groovy, and Python. As we add more GLVs, we can add more string mappers. Any other solution thus far has either been too lenient or too restrictive. Added more test cases to LambdaRestrictionStrategyTest as well. ---
[GitHub] tinkerpop issue #729: TINKERPOP-1632: Create a set of default functions
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/729 I just unzipped the jar and the META-INF/ does not contain it. Just a pom.xml in there. ---
[GitHub] tinkerpop issue #729: TINKERPOP-1632: Create a set of default functions
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/729 ``` [INFO] [INFO] BUILD SUCCESS [INFO] [INFO] Total time: 02:34 h [INFO] Finished at: 2017-10-04T16:21:53-06:00 [INFO] Final Memory: 165M/1543M [INFO] ``` ---
[GitHub] tinkerpop pull request #729: TINKERPOP-1632: Create a set of default functio...
Github user okram commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/729#discussion_r142803300
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MathStep.java
---
@@ -0,0 +1,171 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.tinkerpop.gremlin.process.traversal.step.map;
+
+import net.objecthunter.exp4j.Expression;
+import net.objecthunter.exp4j.ExpressionBuilder;
+import org.apache.tinkerpop.gremlin.process.traversal.Pop;
+import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
+import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
+import org.apache.tinkerpop.gremlin.process.traversal.step.ByModulating;
+import org.apache.tinkerpop.gremlin.process.traversal.step.PathProcessor;
+import org.apache.tinkerpop.gremlin.process.traversal.step.Scoping;
+import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
+import
org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement;
+import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalRing;
+import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil;
+import org.apache.tinkerpop.gremlin.structure.util.StringFactory;
+
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * @author Marko A. Rodriguez (http://markorodriguez.com)
+ */
+public final class MathStep extends MapStep implements
ByModulating, TraversalParent, Scoping, PathProcessor {
+
+private static final String CURRENT = "_";
+private final String equation;
+private final Set variables;
+private TraversalRing traversalRing = new
TraversalRing<>();
+private Set keepLabels;
+
+public MathStep(final Traversal.Admin traversal, final String
equation) {
+super(traversal);
+this.equation = equation;
+this.variables = MathStep.getVariables(this.equation);
+
+}
+
+@Override
+protected Traverser.Admin processNextStart() {
+return
PathProcessor.processTraverserPathLabels(super.processNextStart(),
this.keepLabels);
+}
+
+@Override
+protected Double map(final Traverser.Admin traverser) {
+final Expression expression = new ExpressionBuilder(this.equation)
--- End diff --
Not with threading. Thus, OLAP and any other execution engine that uses
threads will get inconsistent results.
---
[GitHub] tinkerpop pull request #729: TINKERPOP-1632: Create a set of default functio...
GitHub user okram opened a pull request:
https://github.com/apache/tinkerpop/pull/729
TINKERPOP-1632: Create a set of default functions
https://issues.apache.org/jira/browse/TINKERPOP-1632
This is an implementation of lambda-less math capabilities in Gremlin
leveraging the String-based calculator model proposed by @twilmes. This model
deviates from Gremlin's standard function composition and nesting model to
provide a easy to read "math processor" that leverages Gremlin scopes for data
access (e.g. path data, map keys, and side-effects). The feature is
encapsulated in `MathStep` which implements `PathProcessor` and `Scoping`.
Furthermore, `by()`-modulation is supported. `by()`-modulators are applied in
the order in which the variable is first used in the equation. Thus:
```
g.V().as("a").out("knows").by("b").
math("b + a").
by(out().count()).
by("age")
```
In the above: `a` -> `age` and `b` -> `out().count()`. More complex
examples are provided below:
```
gremlin> g = TinkerFactory.createModern().traversal()
==>graphtraversalsource[tinkergraph[vertices:6 edges:6], standard]
gremlin> g.V().as('a').out('knows').as('b').math('a + b').by('age')
==>56.0
==>61.0
gremlin> g.V().as('a').out('created').as('b').
..1> math('b + a').
..2> by(both().count().math('_ + 100')).
..3> by('age')
==>132.0
==>133.0
==>135.0
==>138.0
gremlin> g.withSideEffect('x',10).V().values('age').math('_ / x')
==>2.9
==>2.7
==>3.2
==>3.5
gremlin>
g.withSack(1).V(1).repeat(sack(sum).by(constant(1))).times(10).emit().sack().math('sin
_')
==>0.9092974268256817
==>0.1411200080598672
==>-0.7568024953079282
==>-0.9589242746631385
==>-0.27941549819892586
==>0.6569865987187891
==>0.9893582466233818
==>0.4121184852417566
==>-0.5440211108893698
==>-0.902065507035
```
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/tinkerpop TINKERPOP-1632
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tinkerpop/pull/729.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #729
commit 6f1a1f7028f285c6e45b7e7596320710b91114c5
Author: Marko A. Rodriguez
Date: 2017-10-03T23:37:10Z
first non-tested implementation of math()-step. It uses the @twilmes model
which parses a String representation of the expression.
commit 624a8d706e150cc15dc2412d66d3ac9a61b90682
Author: Marko A. Rodriguez
Date: 2017-10-04T16:27:48Z
I wrote a small parser that is able to extract variables from an exp4j
equation. So far, it is pretty durable. This is a much cleaner way of
determining variables than via label, side-effect, and map analysis. However,
this means that the by()-modulations are with respects to the order in which
the variables are contained in the equation.
commit c36493b6d66406b0d6b50e6c292e6d43216b4c4c
Author: Marko A. Rodriguez
Date: 2017-10-04T16:42:06Z
added MathTest to test math() using step labels, side-effects, and implicit
current -- and with various uses of by()-modulation. This is a really cool step.
commit dfddccaaa85898e046e6b7a5bd81da3d94777d7d
Author: Marko A. Rodriguez
Date: 2017-10-04T17:01:53Z
added MathStepTest to test hashCode() and the custom variable finder. Found
a couple of problems in my parser and I fixed them. Forgot to make MathStep a
PathProcessor so that OLAP is smart about data access. Both OLTP and OLAP tests
pass.
commit 7217823c3ea73f2d32ffa975e417dcd49d40cbdd
Author: Marko A. Rodriguez
Date: 2017-10-04T17:30:55Z
added math()-step to the reference docs and updated CHANGELOG and upgrade
docs.
---
[GitHub] tinkerpop pull request #721: TINKERPOP-1786 Recipe and missing manifest item...
Github user okram commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/721#discussion_r142426106 --- Diff: hadoop-gremlin/conf/hadoop-gryo.properties --- @@ -29,8 +29,8 @@ gremlin.hadoop.outputLocation=output spark.master=local[4] spark.executor.memory=1g spark.serializer=org.apache.tinkerpop.gremlin.spark.structure.io.gryo.GryoSerializer +gremlin.spark.persistContext=true --- End diff -- Yes, we know what it does, but by default we have it set to `false` and we don't want to create a backwards breaking usage. ---
[GitHub] tinkerpop pull request #726: TINKERPOP-1795: Getting Lambda comparator messa...
GitHub user okram opened a pull request: https://github.com/apache/tinkerpop/pull/726 TINKERPOP-1795: Getting Lambda comparator message for .profile() step https://issues.apache.org/jira/browse/TINKERPOP-1795 Some objects are hard to check for "lambdaness" and require a `toString()` analysis. We checked for `@` but that doesn't work for named steps within `order()` (when using `profile()`). That is, its error prone. I came up with a new way to check for lambdas that works for Java, Groovy, and Python/Jython. @spmallette -- can you please verify that a "toString()" of a lambda in C# will hold as well for the pattern. VOTE +1 You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1795 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/726.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #726 commit beae74c43505d1f7732f92a500dc58fc4b142af1 Author: Marko A. Rodriguez Date: 2017-10-02T21:23:33Z fixed a bug in LambdaRestrictionStrategy where named @ steps were considered lambda. Came up with a different way to check lambdas in .toString(). Works for Groovy, Java, and Python. ---
[GitHub] tinkerpop pull request #721: TINKERPOP-1786 Recipe and missing manifest item...
Github user okram commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/721#discussion_r142035586 --- Diff: hadoop-gremlin/conf/hadoop-gryo.properties --- @@ -29,8 +29,8 @@ gremlin.hadoop.outputLocation=output spark.master=local[4] spark.executor.memory=1g spark.serializer=org.apache.tinkerpop.gremlin.spark.structure.io.gryo.GryoSerializer +gremlin.spark.persistContext=true --- End diff -- Why is this defaulted now? ---
[GitHub] tinkerpop issue #723: TINKERPOP-1792 Fixed GremlinScriptEngine bug in lambda...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/723 VOTE +1 ---
[GitHub] tinkerpop issue #724: TINKERPOP-1793: addE() should allow dynamic edge label...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/724 ``` [INFO] [INFO] BUILD SUCCESS [INFO] [INFO] Total time: 02:21 h [INFO] Finished at: 2017-09-27T14:16:02-06:00 [INFO] Final Memory: 170M/1515M [INFO] ``` ---
[GitHub] tinkerpop pull request #724: TINKERPOP-1793: addE() should allow dynamic edg...
GitHub user okram opened a pull request: https://github.com/apache/tinkerpop/pull/724 TINKERPOP-1793: addE() should allow dynamic edge labels https://issues.apache.org/jira/browse/TINKERPOP-1793 There are a few changes here: 1. `addV(traversal)` and `addE(traversal)` added to both `GraphTraversal` and `GraphTraversalSource`. Thus, the label of the respectively created element can be dynamically determined. 2. Added the above methods as well as `addE(String)` to the traversal source of `GremlinDslProcessor`. The missing `addE(String)` method was a bug. 3. Changed `to()` and `from()` from taking a `Traversal` to `Traversal`. This reduces type coercion. Added various test cases to validate proper functioning. VOTE +1. You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1793 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/724.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #724 commit bd2c3b27059a50b87960a5bf270c5b9633bb8edc Author: Marko A. Rodriguez Date: 2017-09-27T16:05:16Z first push of working addV(traversal) and addE(traversal). I have only written one test case thus far. However, taking a break and pushing what I have. Need to write about 3 more test cases to be confident. commit f0dc7ff5d72e4ba8532326fe9b99c35b476f4b09 Author: Marko A. Rodriguez Date: 2017-09-27T16:55:56Z added GraphTraversalSource.addV(traversal) and GraphTraversalSource.addE(traversal). Added 2 more test cases. One more to go. Updated upgrade docs accordingly. commit 8ff6bdf44d6a3bc922723c0646b5ad39c3c80268 Author: Marko A. Rodriguez Date: 2017-09-27T17:36:53Z changed the typing of from() and to() to accept wildcard instead of E as the start. this reduces nasty type coersion. Added final addE() test case. Updated GremlinDSLProcessor to handle g.addE() and new addV()/addE() traversal methods accordingly. commit 1f76f1e9b1bb3bd980cd3b143228a2c324220477 Author: Marko A. Rodriguez Date: 2017-09-27T17:54:17Z updated upgrade docs. ---
[GitHub] tinkerpop issue #714: TINKERPOP-1782 RangeByIsCountStrategy doesn't handle f...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/714 VOTE +1 ---
[GitHub] tinkerpop issue #721: TINKERPOP-1786 Recipe and missing manifest items for S...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/721 This is good stuff. Is it possible to write a test case for the work? Also, can you fix your documentation so its `TinkerPop` not `Tinkerpop` (camel case). ---
[GitHub] tinkerpop pull request #720: TINKERPOP-1789: Reference elements should be re...
Github user okram commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/720#discussion_r140513636 --- Diff: docs/src/upgrade/release-3.2.x-incubating.asciidoc --- @@ -95,6 +95,19 @@ In `gremlin-test` there is a new `GraphHelper` class that has a `cloneElements() the first graph to the second - `GraphHelper.cloneElements(Graph original, Graph clone)`. This helper method is primarily intended for use in tests. +Upgrading for Providers +~~~ + +ReferenceVertex Label +^ + +`ReferenceVertex.label()` was hard coded to return `EMPTY_STRING`. At some point, `ReferenceElements` were support to --- End diff -- fixed. ---
[GitHub] tinkerpop issue #720: TINKERPOP-1789: Reference elements should be represent...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/720 ``` [INFO] [INFO] BUILD SUCCESS [INFO] [INFO] Total time: 03:23 h [INFO] Finished at: 2017-09-21T18:12:15-06:00 [INFO] Final Memory: 163M/1668M [INFO] --- ``` ---
[GitHub] tinkerpop pull request #720: TINKERPOP-1789: Reference elements should be re...
GitHub user okram opened a pull request: https://github.com/apache/tinkerpop/pull/720 TINKERPOP-1789: Reference elements should be represented by id and label https://issues.apache.org/jira/browse/TINKERPOP-1789 Fixed a bug where `ReferenceVertex` was always returning an `EMPTY_STRING` `label()`. `ReferenceEdge` and `ReferenceVertexProperty` were behaving correctly, but I needed to make a change at `ReferenceElement` for general handling of all element labels. Thus, those other two classes changed. And thus, the Gryo. I updated all the `ReferenceXXXTest` classes to check for `label()` accordingly. Updated CHANGELOG and upgrade docs. VOTE +1 You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1789 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/720.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #720 commit d6cb13e8fb1ff37b354d31cdc96aa5a765538e02 Author: Marko A. Rodriguez Date: 2017-09-21T19:23:15Z Fixed a bug where ReferenceVertex was not returning label() (only EMPTY_STRING). ReferenceEdge and ReferenceVertexProperty were behaving correctly, but I needed to make a change at ReferenceElement for general handling. Thus, those other two classes changed. And thus, the Gryo. I updated all the ReferenceXXXTest classes to check for label() to be certain of proper data handling. Updated CHANGELOG and upgrade docs. commit acbc18ddc37a6a9453d672a0b4f64bcace9cf6cf Author: Marko A. Rodriguez Date: 2017-09-21T19:31:18Z In GraphComputer, labels of adjacent vertices can not be accessed. Thus, ReferenceElement needs to ensure that label() doesn't throw an UnsupportedOperationException. I borrowed the same technique used in DeatchedElement. ---
[GitHub] tinkerpop pull request #717: TINKERPOP-1783: PageRank gives incorrect result...
Github user okram commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/717#discussion_r139994557
--- Diff:
gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/computer/ranking/pagerank/PageRankVertexProgramTest.java
---
@@ -38,7 +38,7 @@
@LoadGraphWith(MODERN)
public void shouldExecutePageRank() throws Exception {
if
(graphProvider.getGraphComputer(graph).features().supportsResultGraphPersistCombination(GraphComputer.ResultGraph.NEW,
GraphComputer.Persist.VERTEX_PROPERTIES)) {
-final ComputerResult result =
graph.compute(graphProvider.getGraphComputer(graph).getClass()).program(PageRankVertexProgram.build().create(graph)).submit().get();
+final ComputerResult result =
graph.compute(graphProvider.getGraphComputer(graph).getClass()).program(PageRankVertexProgram.build().iterations(30).create(graph)).submit().get();
--- End diff --
Pushed. Added a test for iterations breaking, epsilon breaking, and energy
conservation.
---
[GitHub] tinkerpop pull request #717: TINKERPOP-1783: PageRank gives incorrect result...
Github user okram commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/717#discussion_r139992366 --- Diff: docs/src/upgrade/release-3.3.x.asciidoc --- @@ -32,6 +32,43 @@ Please see the link:https://github.com/apache/tinkerpop/blob/3.3.1/CHANGELOG.asc Upgrading for Users ~~~ +PageRankVertexProgram +^ + +There were two major bugs in the way in which PageRank was being calculated in `PageRankVertexProgram`. First, teleportation +energy was not being distributed correctly amongst the vertices at each round. Second, terminal vertices (i.e. vertices +with no outgoing edges) did not have their full gathered energy distributed via teleportation. + +For users upgrading, note that while the relative rank orders will remain "the same," the actual PageRank values will differ +from prior TinkerPop versions. + +``` +VERTEX iGRAPHTINKERPOP +marko 0.1119788 0.11375485828040575 +vadas 0.1370267 0.14598540145985406 +lop 0.2665600 0.30472082661863686 +josh0.1620746 0.14598540145985406 +ripple 0.2103812 0.1757986539008437 +peter 0.1119788 0.11375485828040575 +``` + +Normalization preserved through computation: + +``` +0.11375485828040575 + +0.14598540145985406 + +0.30472082661863686 + +0.14598540145985406 + +0.1757986539008437 + +0.11375485828040575 +==>1.00018 +``` + +Two other additions to `PageRankVertexProgram` were provided as well. + +1. It now calculates the vertex count and thus, no longer requires the user to specify the vertex count. +2. It now allows the user to leverage an epsilon-based convergence instead of having to specify the number of iterations to execute. + --- End diff -- Done. ---
[GitHub] tinkerpop issue #717: TINKERPOP-1783: PageRank gives incorrect results for g...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/717 Pushed `maxIterations` concept. ---
[GitHub] tinkerpop issue #717: TINKERPOP-1783: PageRank gives incorrect results for g...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/717 You know what we should do. It should be `maxIterations`. This way, if it converges before hand, great. Done. If it doesn't, well, it will go for `maxIterations` and then stop. This way `epsilon` and `maxIterations` are not mutually exclusive settings. ---
[GitHub] tinkerpop issue #717: TINKERPOP-1783: PageRank gives incorrect results for g...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/717 @spmallette -- I'm putting this into `TP 3.3.x` because it is a breaking change. I believe it is too late into `3.2.x` to introduce such changes. ---
[GitHub] tinkerpop pull request #717: TINKERPOP-1783: PageRank gives incorrect result...
Github user okram commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/717#discussion_r139985206
--- Diff:
gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/computer/ranking/pagerank/PageRankVertexProgramTest.java
---
@@ -38,7 +38,7 @@
@LoadGraphWith(MODERN)
public void shouldExecutePageRank() throws Exception {
if
(graphProvider.getGraphComputer(graph).features().supportsResultGraphPersistCombination(GraphComputer.ResultGraph.NEW,
GraphComputer.Persist.VERTEX_PROPERTIES)) {
-final ComputerResult result =
graph.compute(graphProvider.getGraphComputer(graph).getClass()).program(PageRankVertexProgram.build().create(graph)).submit().get();
+final ComputerResult result =
graph.compute(graphProvider.getGraphComputer(graph).getClass()).program(PageRankVertexProgram.build().iterations(30).create(graph)).submit().get();
--- End diff --
There are plenty of `pageRank()`-tests with some testing convergence and
some testing `times()`. See `PageRankTest`. What other test do you have in
mind?
---
[GitHub] tinkerpop pull request #717: TINKERPOP-1783: PageRank gives incorrect result...
Github user okram commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/717#discussion_r139984956
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/ranking/pagerank/PageRankVertexProgram.java
---
@@ -155,61 +161,61 @@ public PageRankVertexProgram clone() {
@Override
public void setup(final Memory memory) {
-memory.set(TELEPORTATION_ENERGY, 0.0d);
+memory.set(TELEPORTATION_ENERGY, null == this.initialRankTraversal
? 1.0d : 0.0d);
memory.set(VERTEX_COUNT, 0.0d);
+memory.set(CONVERGENCE_ERROR, 1.0d);
}
@Override
public void execute(final Vertex vertex, Messenger messenger,
final Memory memory) {
if (memory.isInitialIteration()) {
messenger.sendMessage(this.countMessageScope, 1.0d);
memory.add(VERTEX_COUNT, 1.0d);
-} else if (1 == memory.getIteration()) {
-final double vertexCount = memory.get(VERTEX_COUNT);
-double initialPageRank = null == this.initialRankTraversal ?
-1.0d / vertexCount :
-TraversalUtil.apply(vertex,
this.initialRankTraversal.get()).doubleValue();
-vertex.property(VertexProperty.Cardinality.single,
this.property, initialPageRank);
-final double edgeCount =
IteratorUtils.reduce(messenger.receiveMessages(), 0.0d, (a, b) -> a + b);
-vertex.property(VertexProperty.Cardinality.single, EDGE_COUNT,
edgeCount);
-memory.add(TELEPORTATION_ENERGY, (1.0d - this.alpha) *
initialPageRank);
-initialPageRank = this.alpha * initialPageRank;
-if (!this.terminate(memory)) { // don't send messages if this
is the last iteration
-if (edgeCount > 0.0d)
-messenger.sendMessage(this.incidentMessageScope,
initialPageRank / edgeCount);
-else
-memory.add(TELEPORTATION_ENERGY, initialPageRank);
-}
} else {
final double vertexCount = memory.get(VERTEX_COUNT);
-double newPageRank =
IteratorUtils.reduce(messenger.receiveMessages(), 0.0d, (a, b) -> a + b);
-final double terminalEnergy = memory.get(TELEPORTATION_ENERGY);
-if (terminalEnergy > 0.0d) {
-final double localTerminalEnergy = terminalEnergy /
vertexCount;
-newPageRank = newPageRank + localTerminalEnergy;
-memory.add(TELEPORTATION_ENERGY, -localTerminalEnergy);
+final double edgeCount;
+double pageRank;
+if (1 == memory.getIteration()) {
+edgeCount =
IteratorUtils.reduce(messenger.receiveMessages(), 0.0d, (a, b) -> a + b);
+vertex.property(VertexProperty.Cardinality.single,
EDGE_COUNT, edgeCount);
+pageRank = null == this.initialRankTraversal ?
+0.0d :
+TraversalUtil.apply(vertex,
this.initialRankTraversal.get()).doubleValue();
+} else {
+edgeCount = vertex.value(EDGE_COUNT);
+pageRank =
IteratorUtils.reduce(messenger.receiveMessages(), 0.0d, (a, b) -> a + b);
}
-vertex.property(VertexProperty.Cardinality.single,
this.property, newPageRank);
-memory.add(TELEPORTATION_ENERGY, (1.0d - this.alpha) *
newPageRank);
-newPageRank = this.alpha * newPageRank;
-final double edgeCount = vertex.value(EDGE_COUNT);
-if (!this.terminate(memory)) { // don't send messages if this
is the last iteration
-if (edgeCount > 0.0d)
-messenger.sendMessage(this.incidentMessageScope,
newPageRank / edgeCount);
-else
-memory.add(TELEPORTATION_ENERGY, newPageRank);
+//
+final double teleporationEnergy =
memory.get(TELEPORTATION_ENERGY);
+if (teleporationEnergy > 0.0d) {
+final double localTerminalEnergy = teleporationEnergy /
vertexCount;
+pageRank = pageRank + localTerminalEnergy;
+memory.add(TELEPORTATION_ENERGY, -localTerminalEnergy);
}
+final double previousPageRank =
vertex.property(this.property).orElse(0.0d);
+memory.add(CONVERGENCE_ERROR, Math.abs(pageRank -
previousPageRank));
+vertex.property(VertexProperty.Cardinality.single,
this.property, pageRank);
+memory.add(TELEPORTATION_ENERGY, (1.0d - this.alpha) *
pageRank);
+pageRank = this.alpha * pageRank;
[GitHub] tinkerpop pull request #717: TINKERPOP-1783: PageRank gives incorrect result...
Github user okram commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/717#discussion_r139984575 --- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/traversal/step/map/PageRankVertexProgramStep.java --- @@ -47,7 +47,7 @@ private PureTraversal edgeTraversal; private String pageRankProperty = PageRankVertexProgram.PAGE_RANK; -private int times = 30; +private int times = -2; --- End diff -- No, cause of how iterations in `GraphComputer` differ from iterations in PageRank. We immediately get a `+1` and I need it to be `-1`, not `0`. ---
[GitHub] tinkerpop issue #713: TINKERPOP-1779 Bump to GMavenPlus 1.6
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/713 VOTE +1 ---
[GitHub] tinkerpop pull request #717: TINKERPOP-1783: PageRank gives incorrect result...
GitHub user okram opened a pull request: https://github.com/apache/tinkerpop/pull/717 TINKERPOP-1783: PageRank gives incorrect results for graphs with sinks https://issues.apache.org/jira/browse/TINKERPOP-1783 There were a couple of problems with `PageRankVertexProgram`. In order to describe the problems, its necessary to explain PageRank. *** Initially, every vertex gets `1.0/vertexCount` amount of energy. Thus, the total energy in the system is 1.0. At every iteration, 0.85 of that energy is distributed amongst adjacent neighbors and 0.15 is distributed amongst all vertices (this is the `alpha` parameter w/ the default being 0.85). If a vertex does not have outgoing edges and it can not distribute its 0.85 energy amongst its neighbors, then all that energy is teleported amongst all vertices. The algorithm above ensures strong connectivity because the 0.15 energy distribution simulates a strongly connected graph via the concept of "teleportation." According to a Markov chain proof on ergodicity in strongly connected graphs, PageRank yields a positive eigenvector. That eigenvector is your rank vector and that rank vector's sum total is always the initial energy the system at iteration 0. *** I fixed three things in this PR: 1. Vertex count is now determined in iteration 0 and no longer needs users to provide a vertex count. 2. Teleportation energy (global energy) is defined via a `Memory` variable. 3. Isolated vertices send their energy to the teleportation variable. Here are the results compared with iGraph's implementation (note that iGraph does a convergence check while TinkerPop does not. It would be extremely expensive to test convergence in a distributed system -- thus, slightly different results). ``` VERTEX iGRAPHTINKERPOP marko 0.1119788 0.11375485828040575 vadas 0.1370267 0.14598540145985406 lop 0.2665600 0.30472082661863686 josh0.1620746 0.14598540145985406 ripple 0.2103812 0.1757986539008437 peter 0.1119788 0.11375485828040575 ``` Finally, a proof of energy conservation: ``` gremlin> 0.11375485828040575 + 0.14598540145985406 + 0.30472082661863686 + 0.14598540145985406 + 0.1757986539008437 + 0.11375485828040575 ==>1.00018 ``` VOTE +1 You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1783 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/717.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #717 commit f7e717de2d4b98ca0a49662f545b1b61ed01becd Author: Marko A. Rodriguez Date: 2017-09-15T20:23:10Z added a way to distributed 'terminal energy' throughout the graph via the implicit teleportation matrix in PageRankVertexProgram. Everthing looks good except that I'm not get the appropriate normalization to 1.0 of the resultant vector and I can't seem to figure out why. Committing what I have so far. commit db7d996bd46978b54573d956935e7eea97ef6338 Author: Marko A. Rodriguez Date: 2017-09-18T16:22:06Z finalized worked on PageRankVertexProgram fix. Really cool use of Memory to solve the 'teleporatation problem.' Proud of myself. Updated CHANGELOG and made a note to users about changing PageRank values in UPGRADE docs. ---
[GitHub] tinkerpop pull request #714: TINKERPOP-1782 RangeByIsCountStrategy doesn't h...
Github user okram commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/714#discussion_r138748892
--- Diff:
gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/RangeByIsCountStrategyTest.java
---
@@ -99,6 +99,9 @@
{__.and(__.out().count().is(0), __.in().count().is(1)),
__.and(__.not(__.out()), __.in().limit(2).count().is(1))},
{__.and(__.out().count().is(1), __.in().count().is(0)),
__.and(__.out().limit(2).count().is(1), __.not(__.in()))},
{__.or(__.out().count().is(0), __.in().count().is(0)),
__.or(__.not(__.out()), __.not(__.in()))},
+{__.path().filter(__.count().is(gte(0.5))).limit(5),
__.path().identity().limit(5)}, // unfortunately we can't just remove the
filter step
--- End diff --
Why can't you remove the `filter()` step? That is, why the `identity()`
step replacement?
---
[GitHub] tinkerpop issue #708: TINKERPOP-1770 Enable timeouts for remote traversals
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/708 VOTE +1 ---
[GitHub] tinkerpop pull request #711: TINKERPOP-1746: Better error message on wrong o...
GitHub user okram opened a pull request: https://github.com/apache/tinkerpop/pull/711 TINKERPOP-1746: Better error message on wrong ordering of emit()/until()/has() Simply added a proper error message when `repeatTraversal` is null. This provides a better user experience than `NullPointerException`. VOTE +1 You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1746 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/711.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #711 commit b28701f7c660c87b05edee3404a66c2fb3671089 Author: Marko A. Rodriguez Date: 2017-09-11T17:56:20Z added better error message for RepeatStep usage errors. ---
[GitHub] tinkerpop issue #705: make TinkerGraph cloneable
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/705
Yes. Exactly.
Here is the thing is -- I don't really care for TinkerPop3 anymore so it
can go as people want it too, I won't fight. But if you want to start aligning
concepts so TinkerPop4 is done "right," we need to start getting away from
methods on `Graph` as users will not have access to `Graph` only `Connections`.
Thus, every action/process is a `Traversal` and thus, all notions of cloning,
clearing, etc. should be via compilation/strategy_introspection of the
`Traversal`.
"But Marko, what about schema stuff like `createIndex`? How will that work
in TinkerPop4" ... That is a vendor specific schema DSL.
`schema.index("name")`, etc.
I think instead of just going "willy nilly" with TinkerPop3, we should
start to make decisions on its design that will align best with TinkerPop4 so
we start to get the right thinking in place now for the big effort to come.
---
[GitHub] tinkerpop issue #705: make TinkerGraph cloneable
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/705 `TinkerGraph.clear()` is bad too. It is `g.V().drop()`. ---
[GitHub] tinkerpop issue #705: make TinkerGraph cloneable
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/705 I don't think it is a good idea to implement `TinkerGraph.clone()`. I think, instead, that there could be a "clone utility" that works for all graphs, not just TinkerGraph. E.g. ``` GraphHelper.cloneGraph(original, new) ``` The reasoning of implementing `clone()` just for test cases is weak and doesn't warrant the semantic confusion that this would entail (as no other graphs are cloneable). ---
[GitHub] tinkerpop issue #699: TINKERPOP-1759 Improve hashcode and equals for Travers...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/699 I'm scared -- VOTE +1. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #701: TINKERPOP-1762: Make MatchStep analyze mid-clau...
Github user okram commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/701#discussion_r136094137
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MatchStep.java
---
@@ -513,8 +513,15 @@ public int hashCode() {
this.scopeKeys = new HashSet<>();
if (null != this.selectKey)
this.scopeKeys.add(this.selectKey);
-if (this.getNextStep() instanceof WhereTraversalStep ||
this.getNextStep() instanceof WherePredicateStep)
-this.scopeKeys.addAll(((Scoping)
this.getNextStep()).getScopeKeys());
+final Set endLabels = ((MatchStep)
this.getTraversal().getParent()).getMatchEndLabels();
+Stream.concat(
+
TraversalHelper.getStepsOfAssignableClassRecursively(WherePredicateStep.class,
this.getTraversal()).stream(),
+
TraversalHelper.getStepsOfAssignableClassRecursively(WhereTraversalStep.class,
this.getTraversal()).stream()).
+flatMap(s ->
s.getScopeKeys().stream()).filter(endLabels::contains).forEach(this.scopeKeys::add);
--- End diff --
Done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #702: TINKERPOP-1764: Generalize MatchStep to localiz...
GitHub user okram opened a pull request:
https://github.com/apache/tinkerpop/pull/702
TINKERPOP-1764: Generalize MatchStep to localize all barriers, not just
reducing barriers.
https://issues.apache.org/jira/browse/TINKERPOP-1764
Prior to this moment, only reducing barrier steps in a `match()`-clause
forced the clause to be computed locally (via a `flatMap()`-wrap). However, it
has been deemed important to generalize this behavior to all barriers (e.g.
`order()`, `limit()`, etc.) as identified by @doanduyhai (Gremlin power user).
As such, the generalization has been done, but at the expense of a breaking
change. However, realize that this breaking change would only be breaking
behavior that is "random" (and most users should not be doing anyways -- but
you never know).
*PREVIOUSLY*
```
gremlin> g.V().match(
..1>
__.as('a').outE('created').order().by('weight',decr).limit(1).inV().as('b'),
..2> __.as('b').has('lang','java')
..3> ).select('a','b').by('name')
==>[a:marko,b:lop]
```
*CURRENTLY*
```
gremlin> g.V().match(
..1>
__.as('a').outE('created').order().by('weight',decr).limit(1).inV().as('b'),
..2> __.as('b').has('lang','java')
..3> ).select('a','b').by('name')
==>[a:marko,b:lop]
==>[a:josh,b:ripple]
==>[a:peter,b:lop]
```
VOTE +1
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/tinkerpop TINKERPOP-1764
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tinkerpop/pull/702.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #702
commit 04b61d5943ecf12338ff15181ff01ba6fd2f143b
Author: Marko A. Rodriguez
Date: 2017-08-30T14:33:34Z
generalized match() to locally compute with all barriers, not just reducing
barriers.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #701: TINKERPOP-1762: Make MatchStep analyze mid-clau...
GitHub user okram opened a pull request:
https://github.com/apache/tinkerpop/pull/701
TINKERPOP-1762: Make MatchStep analyze mid-clause variables for executing
ordering purposes.
https://issues.apache.org/jira/browse/TINKERPOP-1762
There was a bug in `MatchStep` where if you had a mid-clause `where()`, its
variable was not being considered as part of the determination of the matching
start-variables. This led to traversers going down clause paths when they
didn't have all the subsequent data.
```
gremlin> graph = TinkerGraph.open()
==>tinkergraph[vertices:0 edges:0]
gremlin> graph.io(graphml()).readGraph("data/grateful-dead.xml")
==>null
gremlin> g = graph.traversal()
==>graphtraversalsource[tinkergraph[vertices:808 edges:8049], standard]
gremlin> g.V().match(
..1> __.as("sunshine").has("song", "name", "HERE COMES SUNSHINE"),
..2>
__.as("sunshine").map(inE("followedBy").values("weight").mean()).as("avg_weight"),
..3> __.as("sunshine").inE("followedBy").as("x"),
..4>
__.as("x").filter(values("weight").where(gte("avg_weight"))).outV().as("followers")).
..5> select("followers").by("name");
==>LOOKS LIKE RAIN
==>PROMISED LAND
==>LIBERTY
==>EL PASO
==>PLAYING IN THE BAND
==>BIG RIVER
==>CASEY JONES
==>THE MUSIC NEVER STOPPED
gremlin>
```
VOTE +1
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/tinkerpop TINKERPOP-1762
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tinkerpop/pull/701.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #701
commit 651367be902a378a6d813ac39efac0bbc20561eb
Author: Marko A. Rodriguez
Date: 2017-08-29T21:46:32Z
fixed a bug in match() where mid-clause where() variables were not being
considered (only start where()s were).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #698: added repeat simple path test
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/698 @xytxytxyt -- please close this ticket if everything looks good to you. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #698: added repeat simple path test
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/698
Merged. I had to tweak both tests a bit. `limit(1)` still in the groovy
test. The test used `values('name')` instead of just `name`. Groovy test was
not in sugar syntax.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #700: TINKERPOP-1760: OLAP compilation failing around Connec...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/700 ``` [INFO] [INFO] BUILD SUCCESS [INFO] [INFO] Total time: 03:21 h [INFO] Finished at: 2017-08-28T16:04:55-06:00 [INFO] Final Memory: 162M/1646M [INFO] ``` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #698: added repeat simple path test
Github user okram commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/698#discussion_r135646159
--- Diff:
gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/branch/GroovyRepeatTest.groovy
---
@@ -97,5 +97,10 @@ public abstract class GroovyRepeatTest {
public Traversal>
get_g_V_repeatXbothX_untilXname_eq_marko_or_loops_gt_1X_groupCount_byXnameX() {
new ScriptTraversal<>(g, "gremlin-groovy",
"g.V.repeat(both()).until{it.get().value('name').equals('lop') || it.loops() >
1}.groupCount.by('name')")
}
+
+@Override
+public Traversal
get_g_V_hasXname_markoX_repeatXoutE_inV_simplePathX_untilXhasXname_rippleXX_path_byXnameX_byXlabelX()
{
+new ScriptTraversal<>(g, "gremlin-groovy", "g.V().has('name',
'marko').repeat(outE().inV().simplePath()).until(has('name',
'ripple')).limit(1).path().by(values('name')).by(T.label)")
--- End diff --
Good find. I'll tweak that accordingly on merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #699: TINKERPOP-1759 Improve hashcode and equals for ...
Github user okram commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/699#discussion_r135642938
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/traverser/B_O_S_SE_SL_Traverser.java
---
@@ -110,17 +112,22 @@ public void merge(final Traverser.Admin other) {
/
+final boolean carriesUnmergeableSack() {
+// hmmm... serialization in OLAP destroys the transient sideEffects
+return null != this.sack && (null == this.sideEffects || null ==
this.sideEffects.getSackMerger());
+}
+
@Override
public int hashCode() {
-return this.t.hashCode() + this.future.hashCode() + this.loops;
+return carriesUnmergeableSack() ? System.identityHashCode(this) :
(super.hashCode() ^ this.loops);
+}
+
+protected final boolean equals(final B_O_S_SE_SL_Traverser other) {
--- End diff --
I'm a little scared of this "double equals()" definition. @spmallette -- do
you have any thoughts on this matter? Note that there is an `equals(Object)`
method like always, but then there is this added `protected final
equals(SpecificClass)`. I have a feeling that this could get screwy for
languages like Groovy. Perhaps there is a better way to do what is needed
without overloading `equals()`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #699: TINKERPOP-1759 Improve hashcode and equals for ...
Github user okram commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/699#discussion_r135642562 --- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/traverser/O_Traverser.java --- @@ -23,6 +23,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.traverser.util.AbstractTraverser; --- End diff -- There is no `hashCode()` method for this class either like `B_O_...`. Again, its just `return t.hashCode()` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #699: TINKERPOP-1759 Improve hashcode and equals for ...
Github user okram commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/699#discussion_r135642343
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/traverser/LP_O_OB_P_S_SE_SL_Traverser.java
---
@@ -106,17 +106,18 @@ public void dropPath() {
@Override
public int hashCode() {
-return super.hashCode() + this.path.hashCode();
+if (carriesUnmergeableSack()) {
--- End diff --
Can you use inline `returns carriesUnmergableSack() ? ... : ...` please?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #699: TINKERPOP-1759 Improve hashcode and equals for ...
Github user okram commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/699#discussion_r135642190
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/traverser/B_O_Traverser.java
---
@@ -62,10 +62,12 @@ public void setStepId(final String stepId) {
this.future = stepId;
}
+protected final boolean equals(final B_O_Traverser other) {
+return super.equals(other) && other.future.equals(this.future);
+}
+
@Override
--- End diff --
This class does not have a `hashCode()` method. It should since it
implements `equals()`. Its easy, its just `this.t.hashCode()`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #700: TINKERPOP-1760: OLAP compilation failing around Connec...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/700 No. I just spaced on selecting `tp32/` and left it defaulted at `master/`. Fixed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #700: TINKERPOP-1760: OLAP compilation failing around...
GitHub user okram opened a pull request:
https://github.com/apache/tinkerpop/pull/700
TINKERPOP-1760: OLAP compilation failing around ConnectiveStrategy
https://issues.apache.org/jira/browse/TINKERPOP-1760
The more general problem was in `ComputerVerificationStrategy`. The problem
was the nested traversals were being analyzed prior to full strategy
compilation. The solution actually simplified the code a bit.
```
gremlin> g = TinkerFactory.createModern().traversal().withComputer()
==>graphtraversalsource[tinkergraph[vertices:6 edges:6], graphcomputer]
gremlin> g.V().where(out("created").and().out("knows")).values("name")
==>marko
```
VOTE +1
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/tinkerpop TINKERPOP-1760
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tinkerpop/pull/700.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #700
commit c167d9ea779ee2229e5cd56e35a37a6fd225ffc9
Author: Marko A. Rodriguez
Date: 2017-08-28T16:58:34Z
tweaked how TraversalParent children are processed in
ComputerVerifiationStrategy. Given the outside-in nature of Gremlin
compilation, we were analyzing child traversals that were, in fact, not
compiled yet. We now check for local star graph issues on a per traversal
level. I can't believe we didn't run into other problems before this.
commit eb1be4dcfb6053ff65aae6805db61e6556c22384
Author: Marko A. Rodriguez
Date: 2017-08-28T17:04:12Z
updated CHANGELOG.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #699: TINKERPOP-1759 Improve hashcode and equals for ...
Github user okram commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/699#discussion_r135576162
--- Diff:
tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/TinkerGraphPerformanceTest.java
---
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.tinkerpop.gremlin.tinkergraph.process.traversal;
+
+import
org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
+import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerGraph;
+import org.apache.tinkerpop.gremlin.util.TimeUtil;
+import org.junit.Test;
+
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.lessThan;
+import static org.junit.Assert.assertThat;
+
+/**
+ * @author Daniel Kuppitz (http://gremlin.guru)
+ */
+public class TinkerGraphPerformanceTest {
+
+/**
+ * Check the traverser's hashcode() implementation if this test fails.
hashcode() should
+ * not generate too many hash collisions.
+ */
+@Test
+public void sacksWithoutMergerShouldBeFast() {
--- End diff --
This should be in a class called `SackStrategyTest` as `SackStrategy` is
what is being analyzed. This would be analogous to how we do timing tests in
`RepeatUnrollStrategyTest`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #698: added repeat simple path test
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/698 VOTE +1. (I can do the final merge when all the votes are tallied) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #698: added repeat simple path test
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/698
The result is the same. However, I would prefer to have "the simplest query
that fails" and thus, no `limit(1)`, but if the DSEGraph traversal strategies
are failing cause of `limit(1)`, then we can include it. Perhaps you could test
to see if you get the right result in DSEGraph without `limit(1)`.
```
graph.io(gryo()).readGraph('data/tinkerpop-modern.kryo')
g = graph.traversal()
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #698: added repeat simple path test
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/698
Is the `limit(1)` necessary for the DSEGraph testing because against MODERN
its the same result?There is only 1 outgoing path between marko and ripple.
```
gremlin> g.V().has('name',
'marko').repeat(outE().inV().simplePath()).until(has('name',
'ripple')).limit(1).path().by(values('name')).by(T.label)
==>[marko,knows,josh,created,ripple]
gremlin> g.V().has('name',
'marko').repeat(outE().inV().simplePath()).until(has('name',
'ripple')).path().by(values('name')).by(T.label)
==>[marko,knows,josh,created,ripple]
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #698: added repeat simple path test
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/698 What is the intention of the test? What are you trying to prove? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #697: TINKERPOP-1753 OrderStep not able to order by non-inte...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/697 VOTE +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #697: TINKERPOP-1753 OrderStep not able to order by non-inte...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/697 Cleaner `if/else` clause. ``` return first instanceof Number && second instanceof Number ? NumberHelper.compare((Number) second, (Number) first) : Comparator.<~>naturalOrder().compare((Comparable) first, (Comparable) second); ``` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #697: TINKERPOP-1753 OrderStep not able to order by non-inte...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/697 Eek. Yea. That `try/catch` sorta sucks. Can you do some performance testing with and without they `try/catch`? Would you mind? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #691: TINKERPOP-1747 Streamline inheritance for gremlin-pyth...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/691 I am not too hip to the Python code anymore, but I do know that adding the GraphSON3 support was a little "sketchy". Thus, if this is a Python-esque improvement upon my non-Python skills, then great. VOTE +1. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #687: TINKERPOP-1742 RangeByIsCountStrategy fails for Connec...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/687 VOTE +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #688: TINKERPOP-1742 RangeByIsCountStrategy fails for Connec...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/688 VOTE +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #687: TINKERPOP-1742 RangeByIsCountStrategy fails for...
Github user okram commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/687#discussion_r132184141
--- Diff:
gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/CountStrategyTest.java
---
@@ -95,6 +95,7 @@
{__.filter(__.bothE().count().is(gte(1))),
__.filter(__.bothE())},
{__.filter(__.bothE().count().is(gt(1))),
__.filter(__.bothE().limit(2).count().is(gt(1)))},
{__.filter(__.bothE().count().is(gte(2))),
__.filter(__.bothE().limit(2).count().is(gte(2)))},
+{__.and(__.out().count().is(0), __.in().count().is(0)),
__.and(__.not(__.out()), __.not(__.in()))}
});
--- End diff --
Perhaps one more test using both `and()` and `or()` and another pattern
such as `is(gt(1))`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #687: TINKERPOP-1742 RangeByIsCountStrategy fails for...
Github user okram commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/687#discussion_r132183920 --- Diff: CHANGELOG.asciidoc --- @@ -143,6 +143,7 @@ TinkerPop 3.2.6 (Release Date: NOT OFFICIALLY RELEASED YET) This release also includes changes from <>. +* Fixed a bug in `RangeByIsCountStrategy`. --- End diff -- Can you be a bit more explicit? -- e.g. "Fixed a bug regarding `ConnectiveSteps` in `RangeByIsCountStrategy`. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #689: TINKERPOP-1743: LambdaRestrictionStrategy does not cat...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/689 VOTE +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #689: TINKERPOP-1743: LambdaRestrictionStrategy does not cat...
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/689
```
gremlin> g = TinkerFactory.createModern().traversal()
==>graphtraversalsource[tinkergraph[vertices:6 edges:6], standard]
gremlin> g = g.withStrategies(LambdaRestrictionStrategy.instance())
==>graphtraversalsource[tinkergraph[vertices:6 edges:6], standard]
gremlin> g.withSack(0).V().limit(1).sack{ v,s -> System.exit(1) }
The provided step contains a lambda bi-function: SackValueStep(lambda)
Type ':help' or ':h' for help.
Display stack trace? [yN]
gremlin>
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #689: TINKERPOP-1743: LambdaRestrictionStrategy does ...
GitHub user okram opened a pull request: https://github.com/apache/tinkerpop/pull/689 TINKERPOP-1743: LambdaRestrictionStrategy does not catch lambdas passed to sack() https://issues.apache.org/jira/browse/TINKERPOP-1743 Fixed a lambda-leak in `SackValueStep` where `BiFunction` must be tested for true lambda status. You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1743 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/689.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #689 commit 1e04d928a0b87f6aa82e77756f5458d62b14a6d9 Author: Marko A. Rodriguez Date: 2017-08-08T16:12:11Z Fixed a lambda-leak in SackValueStep where BiFunction could be a real lambda. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #685: TINKERPOP-1724 Remove deprecated ScriptElementFactory
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/685 Very good. VOTE +1. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #686: TINKERPOP-1740: Add vertex parameter overload to to() ...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/686 VOTE +1. ``` [INFO] [INFO] BUILD SUCCESS [INFO] [INFO] Total time: 02:15 h [INFO] Finished at: 2017-08-02T19:52:00-06:00 [INFO] Final Memory: 162M/1731M [INFO] ``` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #686: TINKERPOP-1740: Add vertex parameter overload t...
GitHub user okram opened a pull request:
https://github.com/apache/tinkerpop/pull/686
TINKERPOP-1740: Add vertex parameter overload to to() and from()
https://issues.apache.org/jira/browse/TINKERPOP-1740
Is it me or is it hot out? Oo...
```
a = g.V(1).next()
b = g.V(2).next()
g.addE('knows').from(a).to(b)
```
We now support `addE()` as a `GraphTraversalSource` method and we support
`to(Vertex)` and `from(Vertex)` which simply default to
`to(__.constant(vertex))` (and `from()` respectively). Gonna vote on it a bit
later, want to get some feedback from @newkek and Jeremy first.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/tinkerpop TINKERPOP-1740
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tinkerpop/pull/686.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #686
commit 0c92687229955fdb13dc4e3e9a51da1f3b95ae97
Author: Marko A. Rodriguez
Date: 2017-08-02T21:59:19Z
added support for g.addE().to().from() as well as to(vertex) and
from(vertex). This makes things a little easier on the eyes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #684: TINKERPOP-1736 Sack step evaluated by groovy interpret...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/684 VOTE +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #683: TINKERPOP-1736 Sack step evaluated by groovy interpret...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/683 VOTE +1 (got rid of BigDecimal typing -- good). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #682: TINKERPOP-1736 Sack step evaluated by groovy interpret...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/682 Ah. I didn't realize that either. BigDecimal is not something we want to support. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #681: TINKERPOP-1535: Bump to support Giraph 1.2.0.
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/681 ``` [INFO] [INFO] BUILD SUCCESS [INFO] [INFO] Total time: 02:21 h [INFO] Finished at: 2017-07-25T16:52:33-06:00 [INFO] Final Memory: 162M/1813M [INFO] ``` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #682: TINKERPOP-1736 Sack step evaluated by groovy interpret...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/682 VOTE +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #681: TINKERPOP-1535: Bump to support Giraph 1.2.0.
GitHub user okram opened a pull request: https://github.com/apache/tinkerpop/pull/681 TINKERPOP-1535: Bump to support Giraph 1.2.0. https://issues.apache.org/jira/browse/TINKERPOP-1535 I bumped Giraph from 1.1.0 to 1.20. It was a pretty straight forward upgrade. There was only a single class that needed some tweaking -- `MessageCombiner` is now an interface in Giraph instead of a class. VOTE +1. You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1535 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/681.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #681 commit f1d628f3fceec24dfc5e2c559fbd2bea214a68a0 Author: Marko A. Rodriguez Date: 2017-07-25T20:21:36Z bumped to support Giraph 1.2.0. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #677: TINKERPOP-1729: Remove deprecated select steps.
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/677 Some reason the asfbot was unable to pick up this merge. This was merged -- closing. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #677: TINKERPOP-1729: Remove deprecated select steps.
Github user okram closed the pull request at: https://github.com/apache/tinkerpop/pull/677 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #677: TINKERPOP-1729: Remove deprecated select steps.
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/677 @dkuppitz Can you review this please? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #679: TINKERPOP-1679: Detached side-effects aren't attached ...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/679 ``` [INFO] [INFO] BUILD SUCCESS [INFO] [INFO] Total time: 03:33 h [INFO] Finished at: 2017-07-20T20:44:06-06:00 [INFO] Final Memory: 160M/1634M [INFO] ``` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
