You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by "xiazcy (via GitHub)" <gi...@apache.org> on 2023/08/15 00:09:37 UTC
[GitHub] [tinkerpop] xiazcy opened a new pull request, #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
xiazcy opened a new pull request, #2205:
URL: https://github.com/apache/tinkerpop/pull/2205
Implemented a set of string functions outlined in the [proposal](https://github.com/apache/tinkerpop/blob/master/docs/src/dev/future/proposal-3-remove-closures.asciidoc#string-manipulation-functions-in-tinkerpop). Note that scopes are not implemented at this point, as consistent with `concat()`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] xiazcy commented on a diff in pull request #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
Posted by "xiazcy (via GitHub)" <gi...@apache.org>.
xiazcy commented on code in PR #2205:
URL: https://github.com/apache/tinkerpop/pull/2205#discussion_r1310489393
##########
gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/AsString.feature:
##########
@@ -0,0 +1,76 @@
+# 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.
+
+@StepClassMap @StepAsString
Review Comment:
~~I added some tests in https://github.com/apache/tinkerpop/pull/2205/commits/8eb1cf99d3393c35ad73c5516e6ac15697dcf973, and I had to add new regex parsing to get string versions of vertex/edge/vertex properties, is that the correct way to approach asserting string results for these elements?~~
Actually found a better way for all string results, added `str[]` for parsing: https://github.com/apache/tinkerpop/pull/2205/commits/153c75be802183362e4fff493164d6201c8edcd0
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on a diff in pull request #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on code in PR #2205:
URL: https://github.com/apache/tinkerpop/pull/2205#discussion_r1295733218
##########
gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/Length.feature:
##########
@@ -0,0 +1,49 @@
+# 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.
+
+@StepClassMap @StepLength
Review Comment:
going back to an earlier comment for the docs, what happens when the traverser is not a string? do we need tests for that?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] xiazcy commented on a diff in pull request #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
Posted by "xiazcy (via GitHub)" <gi...@apache.org>.
xiazcy commented on code in PR #2205:
URL: https://github.com/apache/tinkerpop/pull/2205#discussion_r1327581743
##########
docs/src/reference/the-traversal.asciidoc:
##########
@@ -1991,6 +2009,22 @@ g.V(1).properties().label()
link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.html#label--++[`label()`]
+[[length-step]]
+=== Length Step
+
+The `length()`-step (*map*) returns the length incoming string traverser. Null values are not processed and remain as null when returned.
Review Comment:
An `IllegalArgumentException` will be thrown (now added in the docs)
##########
docs/src/reference/the-traversal.asciidoc:
##########
@@ -4243,6 +4277,37 @@ link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gre
link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.html#toV-org.apache.tinkerpop.gremlin.structure.Direction-++[`toV(Direction)`],
link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/structure/Direction.html++[`Direction`]
+[[toLower-step]]
+=== ToLower Step
+
+The `toLower()`-step (*map*) returns the lower representation of incoming string traverser. Null values are not processed and remain as null when returned.
+
+[gremlin-groovy,modern]
+----
+g.inject("HELLO", "wORlD", null).toLower()
+----
+
+*Additional References*
+
+link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.html#toLower--++[`toLower()`]
+
+[[toUpper-step]]
+=== ToUpper Step
+
+The `toUpper()`-step (*map*) returns the upper representation of incoming string traverser. Null values are not processed and remain as null when returned.
+
+[gremlin-groovy,modern]
+----
+g.inject("hello", "wORlD", null).toUpper()
+g.V().values("name").toUpper() <1>
+----
+
+<1> Returns the upper case representation of all vertex names.
+
+*Additional References*
+
+link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.html#toLower--++[`toLower()`]
Review Comment:
Fixed
##########
docs/src/upgrade/release-3.7.x.asciidoc:
##########
@@ -29,7 +29,10 @@ complete list of all the modifications that are part of this release.
=== Upgrading for Users
+==== String Manipulation Steps
Review Comment:
Updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] xiazcy commented on a diff in pull request #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
Posted by "xiazcy (via GitHub)" <gi...@apache.org>.
xiazcy commented on code in PR #2205:
URL: https://github.com/apache/tinkerpop/pull/2205#discussion_r1310489393
##########
gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/AsString.feature:
##########
@@ -0,0 +1,76 @@
+# 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.
+
+@StepClassMap @StepAsString
Review Comment:
~~I added some tests in https://github.com/apache/tinkerpop/pull/2205/commits/8eb1cf99d3393c35ad73c5516e6ac15697dcf973, and I had to add new regex parsing to get string versions of vertex/edge/vertex properties, is that the correct way to approach asserting string results for these elements?~~
Actually found a better way for all string results, added `str[]` for parsing: https://github.com/apache/tinkerpop/pull/2205/commits/7f9cee58d74d8a512693cff188102f94df15f2b5
##########
gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/AsString.feature:
##########
@@ -0,0 +1,76 @@
+# 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.
+
+@StepClassMap @StepAsString
Review Comment:
~~I added some tests in https://github.com/apache/tinkerpop/pull/2205/commits/8eb1cf99d3393c35ad73c5516e6ac15697dcf973, and I had to add new regex parsing to get string versions of vertex/edge/vertex properties, is that the correct way to approach asserting string results for these elements?~~
Actually found a better way for all string results, added `str[]` for parsing: https://github.com/apache/tinkerpop/pull/2205/commits/7f9cee58d74d8a512693cff188102f94df15f2b5
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on a diff in pull request #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on code in PR #2205:
URL: https://github.com/apache/tinkerpop/pull/2205#discussion_r1295724192
##########
docs/src/reference/the-traversal.asciidoc:
##########
@@ -1991,6 +2009,22 @@ g.V(1).properties().label()
link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.html#label--++[`label()`]
+[[length-step]]
+=== Length Step
+
+The `length()`-step (*map*) returns the length incoming string traverser. Null values are not processed and remain as null when returned.
Review Comment:
what does `length()` do with non-string types? i suppose this question also applies to `toLower()` and `toUpper()` as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on a diff in pull request #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on code in PR #2205:
URL: https://github.com/apache/tinkerpop/pull/2205#discussion_r1295725799
##########
docs/src/reference/the-traversal.asciidoc:
##########
@@ -4243,6 +4277,37 @@ link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gre
link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.html#toV-org.apache.tinkerpop.gremlin.structure.Direction-++[`toV(Direction)`],
link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/structure/Direction.html++[`Direction`]
+[[toLower-step]]
+=== ToLower Step
+
+The `toLower()`-step (*map*) returns the lower representation of incoming string traverser. Null values are not processed and remain as null when returned.
+
+[gremlin-groovy,modern]
+----
+g.inject("HELLO", "wORlD", null).toLower()
+----
+
+*Additional References*
+
+link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.html#toLower--++[`toLower()`]
+
+[[toUpper-step]]
+=== ToUpper Step
+
+The `toUpper()`-step (*map*) returns the upper representation of incoming string traverser. Null values are not processed and remain as null when returned.
+
+[gremlin-groovy,modern]
+----
+g.inject("hello", "wORlD", null).toUpper()
+g.V().values("name").toUpper() <1>
+----
+
+<1> Returns the upper case representation of all vertex names.
+
+*Additional References*
+
+link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.html#toLower--++[`toLower()`]
Review Comment:
still linking to toLower
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] kenhuuu commented on a diff in pull request #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
Posted by "kenhuuu (via GitHub)" <gi...@apache.org>.
kenhuuu commented on code in PR #2205:
URL: https://github.com/apache/tinkerpop/pull/2205#discussion_r1319158478
##########
gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AsStringStepTest.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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 org.apache.tinkerpop.gremlin.process.traversal.Traversal;
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
+import org.apache.tinkerpop.gremlin.process.traversal.step.StepTest;
+import org.junit.Test;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+/**
+ * @author Yang Xia (http://github.com/xiazcy)
+ */
+public class AsStringStepTest extends StepTest {
+
+ @Override
+ protected List<Traversal> getTraversals() {
+ return Collections.singletonList(__.asString());
+ }
+
+ @Test
+ public void testReturnTypes() {
+ assertEquals("1", __.__(1).asString().next());
+ assertEquals("[1, 2]", __.__(Arrays.asList(1, 2)).asString().next());
+ assertEquals("1", __.__(Arrays.asList(1, 2)).unfold().asString().next());
+ assertArrayEquals(new String[]{"1", "2"}, __.inject(Arrays.asList(1, 2)).unfold().asString().toList().toArray());
+
+ assertEquals("null", __.__(null).asString().next());
Review Comment:
Question: Does it make sense for null to be turned to "null" or just passed along as a null?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] xiazcy commented on a diff in pull request #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
Posted by "xiazcy (via GitHub)" <gi...@apache.org>.
xiazcy commented on code in PR #2205:
URL: https://github.com/apache/tinkerpop/pull/2205#discussion_r1327584947
##########
gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AsStringStepTest.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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 org.apache.tinkerpop.gremlin.process.traversal.Traversal;
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
+import org.apache.tinkerpop.gremlin.process.traversal.step.StepTest;
+import org.junit.Test;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+/**
+ * @author Yang Xia (http://github.com/xiazcy)
+ */
+public class AsStringStepTest extends StepTest {
+
+ @Override
+ protected List<Traversal> getTraversals() {
+ return Collections.singletonList(__.asString());
+ }
+
+ @Test
+ public void testReturnTypes() {
+ assertEquals("1", __.__(1).asString().next());
+ assertEquals("[1, 2]", __.__(Arrays.asList(1, 2)).asString().next());
+ assertEquals("1", __.__(Arrays.asList(1, 2)).unfold().asString().next());
+ assertArrayEquals(new String[]{"1", "2"}, __.inject(Arrays.asList(1, 2)).unfold().asString().toList().toArray());
+
+ assertEquals("null", __.__(null).asString().next());
Review Comment:
I think either works? I've decided to go with the "null" to follow the underlying implementation of the Java `String.valueOf`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] FlorianHockmann commented on a diff in pull request #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
Posted by "FlorianHockmann (via GitHub)" <gi...@apache.org>.
FlorianHockmann commented on code in PR #2205:
URL: https://github.com/apache/tinkerpop/pull/2205#discussion_r1304052705
##########
gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GraphTraversal.cs:
##########
@@ -203,6 +203,15 @@ private GraphTraversal(ICollection<ITraversalStrategy> traversalStrategies, Byte
Bytecode.AddStep("as", args.ToArray());
return Wrap<TStart, TEnd>(this);
}
+
+ /// <summary>
+ /// Adds the asString step to this <see cref="GraphTraversal{SType, EType}" />.
+ /// </summary>
+ public GraphTraversal<TStart, TEnd> AsString<TNewEnd> ()
Review Comment:
`TNewEnd` isn't used so it can be completely removed.
##########
gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GraphTraversal.cs:
##########
@@ -203,6 +203,15 @@ private GraphTraversal(ICollection<ITraversalStrategy> traversalStrategies, Byte
Bytecode.AddStep("as", args.ToArray());
return Wrap<TStart, TEnd>(this);
}
+
+ /// <summary>
+ /// Adds the asString step to this <see cref="GraphTraversal{SType, EType}" />.
+ /// </summary>
+ public GraphTraversal<TStart, TEnd> AsString<TNewEnd> ()
Review Comment:
Can't this return `GraphTraversal<TStart, string>`?
The types should usually be the same as in Java.
##########
gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GraphTraversal.cs:
##########
@@ -1932,6 +1950,24 @@ private GraphTraversal(ICollection<ITraversalStrategy> traversalStrategies, Byte
Bytecode.AddStep("toE", args.ToArray());
return Wrap<TStart, Edge>(this);
}
+
+ /// <summary>
+ /// Adds the toLower step to this <see cref="GraphTraversal{SType, EType}" />.
+ /// </summary>
+ public GraphTraversal<TStart, TNewEnd> ToLower<TNewEnd> ()
+ {
+ Bytecode.AddStep("toLower");
+ return Wrap<TStart, TNewEnd>(this);
+ }
+
+ /// <summary>
+ /// Adds the toUpper step to this <see cref="GraphTraversal{SType, EType}" />.
+ /// </summary>
+ public GraphTraversal<TStart, TNewEnd> ToUpper<TNewEnd> ()
Review Comment:
end here
##########
gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GraphTraversal.cs:
##########
@@ -1932,6 +1950,24 @@ private GraphTraversal(ICollection<ITraversalStrategy> traversalStrategies, Byte
Bytecode.AddStep("toE", args.ToArray());
return Wrap<TStart, Edge>(this);
}
+
+ /// <summary>
+ /// Adds the toLower step to this <see cref="GraphTraversal{SType, EType}" />.
+ /// </summary>
+ public GraphTraversal<TStart, TNewEnd> ToLower<TNewEnd> ()
Review Comment:
Same here
##########
gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GraphTraversal.cs:
##########
@@ -1115,6 +1124,15 @@ private GraphTraversal(ICollection<ITraversalStrategy> traversalStrategies, Byte
Bytecode.AddStep("label");
return Wrap<TStart, string>(this);
}
+
+ /// <summary>
+ /// Adds the length step to this <see cref="GraphTraversal{SType, EType}" />.
+ /// </summary>
+ public GraphTraversal<TStart, TEnd> Length<TNewEnd> ()
Review Comment:
In Java this returns `Number`, but isn't this usually an `int`? In that case, we could change this to:
```suggestion
public GraphTraversal<TStart, int> Length()
```
##########
gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs:
##########
@@ -470,6 +470,10 @@ public static void InstantiateTranslationsForTestRun()
{"g_addVXpersonX_propertyXname_joshX_propertyXage_nullX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.AddV("person").Property("name","josh").Property("age",null), (g,p) =>g.V().Has("person","age",(object) null)}},
{"g_addVXpersonX_propertyXname_markoX_propertyXfriendWeight_null_acl_nullX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.AddV("person").Property("name","marko").Property("friendWeight",null,"acl",null), (g,p) =>g.V().Has("person","name","marko").Has("friendWeight",(object) null), (g,p) =>g.V().Has("person","name","marko").Properties<object>("friendWeight").Has("acl",(object) null), (g,p) =>g.V().Has("person","name","marko").Properties<object>("friendWeight").Count()}},
{"g_V_hasXperson_name_aliceX_propertyXsingle_age_unionXage_constantX1XX_sumX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.AddV("person").Property("name","alice").Property(Cardinality.Single,"age",50), (g,p) =>g.V().Has("person","name","alice").Property("age",__.Union<object>(__.Values<object>("age"),__.Constant<object>(1)).Sum<object>()), (g,p) =>g.V().Has("person","age",50), (g,p) =>g.V().Has("person","age",51)}},
+ {"g_injectX1_2X_asString", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.Inject(1,2).AsString<object>()}},
Review Comment:
This shows what I meant in the other comment. It shouldn't be necessary to provide a type argument here to `AsString()` as that should always return a string.
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/DotNetTranslator.java:
##########
@@ -510,6 +510,7 @@ static final class SymbolHelper {
private final static Map<String, String> FROM_CS_MAP = new HashMap<>();
static {
+ TO_CS_MAP.put(GraphTraversal.Symbols.asString, "AsString<object>");
Review Comment:
If we don't need these added steps to be generic in C#, then we also don't need to add them to this map here since the translator can just translate `asString` to `AsString`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] kenhuuu commented on pull request #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
Posted by "kenhuuu (via GitHub)" <gi...@apache.org>.
kenhuuu commented on PR #2205:
URL: https://github.com/apache/tinkerpop/pull/2205#issuecomment-1710824493
VOTE +1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] Cole-Greer commented on a diff in pull request #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on code in PR #2205:
URL: https://github.com/apache/tinkerpop/pull/2205#discussion_r1318867063
##########
docs/src/reference/the-traversal.asciidoc:
##########
@@ -4243,6 +4278,39 @@ link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gre
link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.html#toV-org.apache.tinkerpop.gremlin.structure.Direction-++[`toV(Direction)`],
link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/structure/Direction.html++[`Direction`]
+[[toLower-step]]
+=== ToLower Step
+
+The `toLower()`-step (*map*) returns the lower representation of incoming string traverser. Null values are not processed and remain as null when returned.
Review Comment:
```suggestion
The `toLower()`-step (*map*) returns the lowercase representation of incoming string traverser. Null values are not processed and remain as null when returned.
```
##########
docs/src/reference/the-traversal.asciidoc:
##########
@@ -4243,6 +4278,39 @@ link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gre
link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.html#toV-org.apache.tinkerpop.gremlin.structure.Direction-++[`toV(Direction)`],
link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/structure/Direction.html++[`Direction`]
+[[toLower-step]]
+=== ToLower Step
+
+The `toLower()`-step (*map*) returns the lower representation of incoming string traverser. Null values are not processed and remain as null when returned.
+If the incoming traverser is a non-String value then an `IllegalArgumentException` will be thrown.
+
+[gremlin-groovy,modern]
+----
+g.inject("HELLO", "wORlD", null).toLower()
+----
+
+*Additional References*
+
+link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.html#toLower--++[`toLower()`]
+
+[[toUpper-step]]
+=== ToUpper Step
+
+The `toUpper()`-step (*map*) returns the upper representation of incoming string traverser. Null values are not processed and remain as null when returned.
Review Comment:
```suggestion
The `toUpper()`-step (*map*) returns the uppercase representation of incoming string traverser. Null values are not processed and remain as null when returned.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on a diff in pull request #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on code in PR #2205:
URL: https://github.com/apache/tinkerpop/pull/2205#discussion_r1295729104
##########
docs/src/upgrade/release-3.7.x.asciidoc:
##########
@@ -29,7 +29,10 @@ complete list of all the modifications that are part of this release.
=== Upgrading for Users
+==== String Manipulation Steps
Review Comment:
this reads a bit too much like a CHANGELOG entry to me - a few more words I think are in order. New steps in upgrade docs should have some examples. i'd also include links in "See" to the reference docs.
also, please add provider docs for these steps.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on a diff in pull request #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on code in PR #2205:
URL: https://github.com/apache/tinkerpop/pull/2205#discussion_r1295724192
##########
docs/src/reference/the-traversal.asciidoc:
##########
@@ -1991,6 +2009,22 @@ g.V(1).properties().label()
link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.html#label--++[`label()`]
+[[length-step]]
+=== Length Step
+
+The `length()`-step (*map*) returns the length incoming string traverser. Null values are not processed and remain as null when returned.
Review Comment:
what does `length()` do with non-string types?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] xiazcy merged pull request #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
Posted by "xiazcy (via GitHub)" <gi...@apache.org>.
xiazcy merged PR #2205:
URL: https://github.com/apache/tinkerpop/pull/2205
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] FlorianHockmann commented on a diff in pull request #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
Posted by "FlorianHockmann (via GitHub)" <gi...@apache.org>.
FlorianHockmann commented on code in PR #2205:
URL: https://github.com/apache/tinkerpop/pull/2205#discussion_r1310135322
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/ToUpperStep.java:
##########
@@ -0,0 +1,60 @@
+/*
+ * 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 org.apache.tinkerpop.gremlin.process.traversal.Traversal;
+import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
+import org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement;
+
+import java.util.Collections;
+import java.util.Set;
+
+/**
+ * Reference implementation for toLower() step, a mid-traversal step which returns an upper case string representation
Review Comment:
```suggestion
* Reference implementation for toUpper() step, a mid-traversal step which returns an upper case string representation
```
##########
docs/src/upgrade/release-3.7.x.asciidoc:
##########
@@ -29,7 +29,68 @@ complete list of all the modifications that are part of this release.
=== Upgrading for Users
+==== String Manipulation Steps
+Additional String manipulation steps have been added to this version: `asString()`, `length()`, `toLower()`, `toUpper()`.
+The following example demonstrates the use of a closure to perform the above functions:
+[source,text]
+----
+gremlin> g.V().hasLabel("person").values("age").map{it.get().toString()}
+==>29
+==>27
+==>32
+==>35
+gremlin> g.V().values("name").map{it.get().length()}
+==>5
+==>5
+==>3
+==>4
+==>6
+==>5
+gremlin> g.inject("TO", "LoWeR", "cAsE").map{it.get().toLowerCase()}
+==>to
+==>lower
+==>case
+gremlin> g.V().values("name").map{it.get().toUpperCase()}
+==>MARKO
+==>VADAS
+==>LOP
+==>JOSH
+==>RIPPLE
+==>PETER
+----
+With these additional steps this operation can be performed with standard Gremlin syntax:
+[source,text]
+----
+gremlin> g.V().hasLabel("person").values("age").asString()
+==>29
+==>27
+==>32
+==>35
+gremlin> g.V().values("name").length()
+==>5
+==>5
+==>3
+==>4
+==>6
+==>5
+gremlin> g.inject("TO", "LoWeR", "cAsE").toLower()
+==>to
+==>lower
+==>case
+gremlin> g.V().values("name").toUpper()
+==>MARKO
+==>VADAS
+==>LOP
+==>JOSH
+==>RIPPLE
+==>PETER
+----
+See: link:https://tinkerpop.apache.org/docs/x.y.z/reference/#asString-step[reference]
Review Comment:
(nitpick) Maybe use different text for these links than just `reference` for every step? Like this, it only shows 4 links with the text "reference". How about "asString()-step" and so on?
That's how it was done for `mergeV()` and `mergeE()`: https://github.com/apache/tinkerpop/blob/8eb1cf99d3393c35ad73c5516e6ac15697dcf973/docs/src/upgrade/release-3.6.x.asciidoc#mergev-and-mergee
##########
gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GraphTraversal.cs:
##########
@@ -1932,6 +1950,24 @@ private GraphTraversal(ICollection<ITraversalStrategy> traversalStrategies, Byte
Bytecode.AddStep("toE", args.ToArray());
return Wrap<TStart, Edge>(this);
}
+
+ /// <summary>
+ /// Adds the toLower step to this <see cref="GraphTraversal{SType, EType}" />.
+ /// </summary>
+ public GraphTraversal<TStart, string> ToLower ()
+ {
+ Bytecode.AddStep("toLower");
+ return Wrap<TStart, string>(this);
+ }
+
+ /// <summary>
+ /// Adds the toUpper step to this <see cref="GraphTraversal{SType, EType}" />.
+ /// </summary>
+ public GraphTraversal<TStart, string> ToUpper ()
Review Comment:
and here:
```suggestion
public GraphTraversal<TStart, string?> ToUpper ()
```
##########
docs/src/reference/the-traversal.asciidoc:
##########
@@ -4243,6 +4278,39 @@ link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gre
link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.html#toV-org.apache.tinkerpop.gremlin.structure.Direction-++[`toV(Direction)`],
link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/structure/Direction.html++[`Direction`]
+[[toLower-step]]
+=== ToLower Step
+
+The `toLower()`-step (*map*) returns the lower representation of incoming string traverser. Null values are not processed and remain as null when returned.
+If the incoming traverser is a non-String value then an `IllegalArgumentException` will be thrown.
+
+[gremlin-groovy,modern]
+----
+g.inject("HELLO", "wORlD", null).toLower()
+----
+
+*Additional References*
+
+link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.html#toLower--++[`toLower()`]
+
+[[toUpper-step]]
+=== ToUpper Step
+
+The `toUpper()`-step (*map*) returns the upper representation of incoming string traverser. Null values are not processed and remain as null when returned.
+If the incoming traverser is a non-String value then an `IllegalArgumentException` will be thrown.
+
+[gremlin-groovy,modern]
+----
+g.inject("hello", "wORlD", null).toUpper()
+g.V().values("name").toUpper() <1>
+----
+
+<1> Returns the upper case representation of all vertex names.
+
+*Additional References*
+
+link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.html#toLower--++[`toUpper()`]
Review Comment:
```suggestion
link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.html#toUpper--++[`toUpper()`]
```
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java:
##########
@@ -1437,6 +1441,57 @@ public default GraphTraversal<S, String> concat(final String... concatStrings) {
return this.asAdmin().addStep(new ConcatStep<>(this.asAdmin(), concatStrings));
}
+ /**
+ * Returns the value of incoming traverser as strings. Null values are returned as a string value "null".
+ *
+ * @return the traversal with an appended {@link AsStringStep}.
+ * @see <a href="http://tinkerpop.apache.org/docs/${project.version}/reference/#concat-step" target="_blank">Reference Documentation - Concat Step</a>
Review Comment:
(nitpick) I think these links should not point to the reference docs of the _concat_ step, right?
##########
gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GraphTraversal.cs:
##########
@@ -1932,6 +1950,24 @@ private GraphTraversal(ICollection<ITraversalStrategy> traversalStrategies, Byte
Bytecode.AddStep("toE", args.ToArray());
return Wrap<TStart, Edge>(this);
}
+
+ /// <summary>
+ /// Adds the toLower step to this <see cref="GraphTraversal{SType, EType}" />.
+ /// </summary>
+ public GraphTraversal<TStart, string> ToLower ()
Review Comment:
Same here:
```suggestion
public GraphTraversal<TStart, string?> ToLower ()
```
##########
gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GraphTraversal.cs:
##########
@@ -1115,6 +1124,15 @@ private GraphTraversal(ICollection<ITraversalStrategy> traversalStrategies, Byte
Bytecode.AddStep("label");
return Wrap<TStart, string>(this);
}
+
+ /// <summary>
+ /// Adds the length step to this <see cref="GraphTraversal{SType, EType}" />.
+ /// </summary>
+ public GraphTraversal<TStart, int> Length ()
Review Comment:
Sorry, but I didn't notice this during my first review of this PR: Since `length()` returns `null` as `null`, this should return `int?` instead of `int`. This communicates that this step can return either an `int` or `null`. (This of course also applies to `__.Length()`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] vkagamlyk commented on pull request #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
Posted by "vkagamlyk (via GitHub)" <gi...@apache.org>.
vkagamlyk commented on PR #2205:
URL: https://github.com/apache/tinkerpop/pull/2205#issuecomment-1710467802
VOTE +1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] codecov-commenter commented on pull request #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #2205:
URL: https://github.com/apache/tinkerpop/pull/2205#issuecomment-1679430628
## [Codecov](https://app.codecov.io/gh/apache/tinkerpop/pull/2205?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#2205](https://app.codecov.io/gh/apache/tinkerpop/pull/2205?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (d3577b7) into [master](https://app.codecov.io/gh/apache/tinkerpop/commit/a68b404f93c4b53cb102986b39115fe0ab54d94e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (a68b404) will **decrease** coverage by `5.47%`.
> The diff coverage is `60.00%`.
```diff
@@ Coverage Diff @@
## master #2205 +/- ##
============================================
- Coverage 75.78% 70.31% -5.47%
============================================
Files 1039 24 -1015
Lines 63502 3561 -59941
Branches 7044 0 -7044
============================================
- Hits 48123 2504 -45619
+ Misses 12763 887 -11876
+ Partials 2616 170 -2446
```
| [Files Changed](https://app.codecov.io/gh/apache/tinkerpop/pull/2205?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [gremlin-go/driver/anonymousTraversal.go](https://app.codecov.io/gh/apache/tinkerpop/pull/2205?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JlbWxpbi1nby9kcml2ZXIvYW5vbnltb3VzVHJhdmVyc2FsLmdv) | `44.30% <0.00%> (-1.55%)` | :arrow_down: |
| [gremlin-go/driver/graphTraversal.go](https://app.codecov.io/gh/apache/tinkerpop/pull/2205?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JlbWxpbi1nby9kcml2ZXIvZ3JhcGhUcmF2ZXJzYWwuZ28=) | `86.24% <100.00%> (+0.37%)` | :arrow_up: |
... and [1015 files with indirect coverage changes](https://app.codecov.io/gh/apache/tinkerpop/pull/2205/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on a diff in pull request #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on code in PR #2205:
URL: https://github.com/apache/tinkerpop/pull/2205#discussion_r1295732396
##########
gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/AsString.feature:
##########
@@ -0,0 +1,76 @@
+# 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.
+
+@StepClassMap @StepAsString
Review Comment:
i think it would be good to include tests for all the graph element types to ensure consistent `toString()` stuff is happening.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] xiazcy commented on a diff in pull request #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
Posted by "xiazcy (via GitHub)" <gi...@apache.org>.
xiazcy commented on code in PR #2205:
URL: https://github.com/apache/tinkerpop/pull/2205#discussion_r1310489393
##########
gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/AsString.feature:
##########
@@ -0,0 +1,76 @@
+# 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.
+
+@StepClassMap @StepAsString
Review Comment:
I added some tests in https://github.com/apache/tinkerpop/pull/2205/commits/8eb1cf99d3393c35ad73c5516e6ac15697dcf973, and I had to add new regex parsing to get string versions of vertex/edge/vertex properties, is that the correct way to approach asserting string results for these elements?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] xiazcy commented on a diff in pull request #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
Posted by "xiazcy (via GitHub)" <gi...@apache.org>.
xiazcy commented on code in PR #2205:
URL: https://github.com/apache/tinkerpop/pull/2205#discussion_r1310482772
##########
gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GraphTraversal.cs:
##########
@@ -1115,6 +1124,15 @@ private GraphTraversal(ICollection<ITraversalStrategy> traversalStrategies, Byte
Bytecode.AddStep("label");
return Wrap<TStart, string>(this);
}
+
+ /// <summary>
+ /// Adds the length step to this <see cref="GraphTraversal{SType, EType}" />.
+ /// </summary>
+ public GraphTraversal<TStart, int> Length ()
Review Comment:
Thank you for all the reviews Florian! I will fix these as needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] xiazcy commented on a diff in pull request #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
Posted by "xiazcy (via GitHub)" <gi...@apache.org>.
xiazcy commented on code in PR #2205:
URL: https://github.com/apache/tinkerpop/pull/2205#discussion_r1327582122
##########
gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/Length.feature:
##########
@@ -0,0 +1,49 @@
+# 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.
+
+@StepClassMap @StepLength
Review Comment:
Tests are added.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] kenhuuu commented on pull request #2205: Implement asString(), length(), toLower(), and toUpper() String Functions
Posted by "kenhuuu (via GitHub)" <gi...@apache.org>.
kenhuuu commented on PR #2205:
URL: https://github.com/apache/tinkerpop/pull/2205#issuecomment-1710824413
This isn't necessary, but it may be a good idea to add tests for empty string.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org