You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by "vaibhavm99 (via GitHub)" <gi...@apache.org> on 2025/05/09 22:47:04 UTC
[PR] getScopingInfo utility added [tinkerpop]
vaibhavm99 opened a new pull request, #3118:
URL: https://github.com/apache/tinkerpop/pull/3118
Added a utility function `getScopingInfo()` in all classes implementing `Scoping` and a method `getScopingInfoForTraversal` to get the labels needed by a traversal, which returns a Set of `ScopingInfo` class which contains the label and Pop info.
<!--
Thanks for contributing! Reminders:
+ TARGET the earliest branch where you want the change
3.7-dev -> 3.7.4 (non-breaking only)
3.8-dev -> 3.8.0
master -> 4.0.0
+ Committers will MERGE the PR forward to newer versions
+ ADD entry to the CHANGELOG.asciidoc for the targeted version
Do not reference a JIRA number there
+ ADD JIRA number to title and link in description
+ PRs requires 3 +1s from committers OR
1 +1 and 7 day wait to merge.
+ MORE details: https://s.apache.org/rtnal
-->
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#issuecomment-2877323350
Hi @vaibhavm99 I'm happy with extending convenience interfaces for providers. I am curious since the `SelectStep`, `SelectOneStep`, and `TraversalSelectStep` are the only steps which are not hardcoded to `Pop.last`, if you also considered unifying these steps with some sort of `Selecting` interface which could extend `Scoping` and add `public Pop getPop();`
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on code in PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#discussion_r2135660038
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/PopContaining.java:
##########
@@ -0,0 +1,104 @@
+/*
+ * 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;
+
+import java.util.HashSet;
+import java.util.Objects;
+
+import org.apache.tinkerpop.gremlin.process.traversal.Pop;
+
+/**
+ * The {@code PopContaining} interface is implemented by traversal steps that maintain Pop instructions
+ * for label access. It provides a mechanism to track and manage how labeled elements should
+ * be accessed using {@link Pop} semantics (first, last, all, or mixed).
+ *
+ * In Gremlin traversals, various elements can be labeled and later accessed via these labels.
+ * The {@link Pop} enum determines how to access these labeled elements when there are multiple
+ * values associated with the same label.
+ *
+ * <pre>
+ * {@code
+ * // Simple example with default Pop.last behavior
+ * gremlin> g.V().as("a").out().as("a").select("a")
+ * ==>[v[2]] // returns the last element labeled "a"
+ *
+ * // Using Pop.first to get the first labeled element
+ * gremlin> g.V().as("a").out().as("a").select(first, "a")
+ * ==>[v[1]] // returns the first element labeled "a"
+ *
+ * // Using Pop.all to get all labeled elements
+ * gremlin> g.V().as("a").out().as("a").select(all, "a")
+ * ==>[v[1], v[2]] // returns all elements labeled "a"
+ * }
+ * </pre>
+ *
+ * Steps implementing this interface maintain a collection of {@link PopInstruction} objects, each containing
+ * a label and a {@link Pop} value that specifies how to access elements with that label.
+ *
+ * @author Vaibhav Malhotra
+ */
+public interface PopContaining {
+ public HashSet<PopInstruction> getPopInstructions();
+ /**
+ * A class for storing the Scope Context. It has two elements:
+ * - label: String
+ * - pop: Pop value
+ */
+ class PopInstruction {
+ public Pop pop;
Review Comment:
shouldn't these be `private final` access member variables?
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on code in PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#discussion_r2107670906
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/PopContaining.java:
##########
@@ -0,0 +1,107 @@
+/*
+ * 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;
+
+import java.util.HashSet;
+import java.util.Objects;
+
+import org.apache.tinkerpop.gremlin.process.traversal.Pop;
+
+/**
+ * The {@code PopContaining} interface is implemented by traversal steps that maintain Pop instructions
+ * for label access. It provides a mechanism to track and manage how labeled elements should
+ * be accessed using {@link Pop} semantics (first, last, all, or mixed).
+ *
+ * In Gremlin traversals, various elements can be labeled and later accessed via these labels.
+ * The {@link Pop} enum determines how to access these labeled elements when there are multiple
+ * values associated with the same label.
+ *
+ * <pre>
+ * {@code
+ * // Simple example with default Pop.last behavior
+ * gremlin> g.V().as("a").out().as("a").select("a")
+ * ==>[v[2]] // returns the last element labeled "a"
+ *
+ * // Using Pop.first to get the first labeled element
+ * gremlin> g.V().as("a").out().as("a").select(first, "a")
+ * ==>[v[1]] // returns the first element labeled "a"
+ *
+ * // Using Pop.all to get all labeled elements
+ * gremlin> g.V().as("a").out().as("a").select(all, "a")
+ * ==>[v[1], v[2]] // returns all elements labeled "a"
+ * }
+ * </pre>
+ *
+ * Steps implementing this interface maintain a collection of {@link PopInstruction} objects, each containing
+ * a label and a {@link Pop} value that specifies how to access elements with that label.
+ *
+ * @author Vaibhav Malhotra
+ */
+public interface PopContaining {
+ public default HashSet<PopInstruction> getPopInstructions() {
Review Comment:
I'm not sure if we want a default implementation which returns an empty set. Are there any steps which will utilize this default behaviour?
If everyone is expected to override this method to return useful/accurate results, I would prefer not to have any default implementation.
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/PopContaining.java:
##########
@@ -0,0 +1,107 @@
+/*
+ * 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;
+
+import java.util.HashSet;
+import java.util.Objects;
+
+import org.apache.tinkerpop.gremlin.process.traversal.Pop;
+
+/**
+ * The {@code PopContaining} interface is implemented by traversal steps that maintain Pop instructions
+ * for label access. It provides a mechanism to track and manage how labeled elements should
+ * be accessed using {@link Pop} semantics (first, last, all, or mixed).
+ *
+ * In Gremlin traversals, various elements can be labeled and later accessed via these labels.
+ * The {@link Pop} enum determines how to access these labeled elements when there are multiple
+ * values associated with the same label.
+ *
+ * <pre>
+ * {@code
+ * // Simple example with default Pop.last behavior
+ * gremlin> g.V().as("a").out().as("a").select("a")
+ * ==>[v[2]] // returns the last element labeled "a"
+ *
+ * // Using Pop.first to get the first labeled element
+ * gremlin> g.V().as("a").out().as("a").select(first, "a")
+ * ==>[v[1]] // returns the first element labeled "a"
+ *
+ * // Using Pop.all to get all labeled elements
+ * gremlin> g.V().as("a").out().as("a").select(all, "a")
+ * ==>[v[1], v[2]] // returns all elements labeled "a"
+ * }
+ * </pre>
+ *
+ * Steps implementing this interface maintain a collection of {@link PopInstruction} objects, each containing
+ * a label and a {@link Pop} value that specifies how to access elements with that label.
+ *
+ * @author Vaibhav Malhotra
+ */
+public interface PopContaining {
+ public default HashSet<PopInstruction> getPopInstructions() {
+ return new HashSet<PopInstruction>();
+ }
+
+ /**
+ * A class for storing the Scope Context. It has two elements:
+ * - label: String
+ * - pop: Pop value
+ */
+ class PopInstruction {
Review Comment:
Nit: this child class is indented 4 spaces too deep
##########
gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectStepTest.java:
##########
@@ -63,4 +67,50 @@ public void shouldRequirePathsAccordingly() {
assertEquals(traversalPath[0], ((Traversal.Admin<?, ?>) traversalPath[1]).getTraverserRequirements().contains(TraverserRequirement.LABELED_PATH));
}
}
+
+ @Test
+ public void testPopInstruction() {
Review Comment:
Would be nice to also add equivalent tests to these for `TraversalSelectStep`
##########
gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/FormatStepTest.java:
##########
@@ -180,4 +183,24 @@ public void shouldPrioritizeVertexPropertiesOverScopeVariables() {
__.__("Marko").as("name").
constant(vertex).format("Hello %{name}").next());
}
+
+ @Test
+ public void testPopInstruction() {
+ final FormatStep formatStep = new FormatStep(__.identity().asAdmin(), "%{Hello} %{world}");
+
+ final PopContaining.PopInstruction popInstruction1 = new PopContaining.PopInstruction();
+ popInstruction1.label = "Hello";
+ popInstruction1.pop = Pop.last;
+
+ final PopContaining.PopInstruction popInstruction2 = new PopContaining.PopInstruction();
+ popInstruction2.label = "world";
+ popInstruction2.pop = Pop.last;
+
+ final HashSet<PopContaining.PopInstruction> popInstructionSet = new HashSet<>();
+ popInstructionSet.add(popInstruction1);
+ popInstructionSet.add(popInstruction2);
+
Review Comment:
Nit: Could use `TestDataBuilder.createPopInstructionSet()`
##########
gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/TestDataBuilder.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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;
+
+import java.util.HashSet;
+
+import org.apache.tinkerpop.gremlin.process.traversal.Pop;
+import org.apache.tinkerpop.gremlin.process.traversal.step.PopContaining;
+
+/**
+ * This class is responsible for building test data for `PopInstruction` Unit tests.
+ * It provides methods to create a set of `PopInstruction` test data objects.
+ *
+ * @author Vaibhav Malhotra
+ */
+public class TestDataBuilder {
+
+ // Helper function to create a Set of `PopInstruction` values
+ public static HashSet<PopContaining.PopInstruction> createPopInstructionSet(Object[]... pairs) {
+ HashSet<PopContaining.PopInstruction> popInstructions = new HashSet<>();
+
+ // Each pair should contain a name (String) and a Pop value
+ for (Object[] pair : pairs) {
+ if (pair.length == 2 && pair[0] instanceof String && pair[1] instanceof Pop) {
+ popInstructions.add(new PopContaining.PopInstruction((String)pair[0], (Pop)pair[1]));
+ }
Review Comment:
Nit: Would be good to add an `else` which throws an `IllegalArgumentException` to identify misuse of this helper.
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on code in PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#discussion_r2087289020
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/FormatStep.java:
##########
@@ -168,6 +169,19 @@ public Set<String> getScopeKeys() {
return variables;
}
+ @Override
+ public Set<ScopingInfo> getScopingInfo() {
Review Comment:
As each step contains a Set of labels, but only one Pop, would it be easier for ScopingInfo to be defined as
```
public static class ScopingInfo {
public Set<String> labels;
public Pop pop;
```
and for this method to return `ScopingInfo` instead of `Set<ScopingInfo>`
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on code in PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#discussion_r2135658365
##########
CHANGELOG.asciidoc:
##########
@@ -30,6 +30,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
[[release-4-0-0-beta-1]]
=== TinkerPop 4.0.0-beta.1 (January 17, 2025)
+* Added an interface `PopContaining` which is implemented by `Scoping` and `TraversalParent`. Used to get label, and Pop info for a step/traversal. The information is wrapped in a `PopInstruction` object
Review Comment:
we tend to like to have a single sentence here. suggesting:
> Added the `PopContaining` interface designed to get label and `Pop` combinations held in a `PopInstruction` object.
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on code in PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#discussion_r2135662153
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/PopContaining.java:
##########
@@ -0,0 +1,104 @@
+/*
+ * 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;
+
+import java.util.HashSet;
+import java.util.Objects;
+
+import org.apache.tinkerpop.gremlin.process.traversal.Pop;
+
+/**
+ * The {@code PopContaining} interface is implemented by traversal steps that maintain Pop instructions
+ * for label access. It provides a mechanism to track and manage how labeled elements should
+ * be accessed using {@link Pop} semantics (first, last, all, or mixed).
+ *
+ * In Gremlin traversals, various elements can be labeled and later accessed via these labels.
+ * The {@link Pop} enum determines how to access these labeled elements when there are multiple
+ * values associated with the same label.
+ *
+ * <pre>
+ * {@code
+ * // Simple example with default Pop.last behavior
+ * gremlin> g.V().as("a").out().as("a").select("a")
+ * ==>[v[2]] // returns the last element labeled "a"
+ *
+ * // Using Pop.first to get the first labeled element
+ * gremlin> g.V().as("a").out().as("a").select(first, "a")
+ * ==>[v[1]] // returns the first element labeled "a"
+ *
+ * // Using Pop.all to get all labeled elements
+ * gremlin> g.V().as("a").out().as("a").select(all, "a")
+ * ==>[v[1], v[2]] // returns all elements labeled "a"
+ * }
+ * </pre>
+ *
+ * Steps implementing this interface maintain a collection of {@link PopInstruction} objects, each containing
+ * a label and a {@link Pop} value that specifies how to access elements with that label.
+ *
+ * @author Vaibhav Malhotra
+ */
+public interface PopContaining {
+ public HashSet<PopInstruction> getPopInstructions();
+ /**
+ * A class for storing the Scope Context. It has two elements:
+ * - label: String
+ * - pop: Pop value
+ */
+ class PopInstruction {
+ public Pop pop;
+ public String label;
+
+ public PopInstruction(String label, Pop pop) {
Review Comment:
nit: the form in TinkerPop code is to make all variables `final` where possible. please give a look over your changes here to see if you can catch any other places where this form might be used.
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "vaibhavm99 (via GitHub)" <gi...@apache.org>.
vaibhavm99 commented on PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#issuecomment-2874891463
> Hi @vaibhavm99, will this change allow to save not the whole Path, but only the parts that are actually used?
Hi @vkagamlyk , I'm sorry I don't understand what you mean by "the whole Path." This change should not have any breaking problems with older code, as it is just adding a new API. This would take in a traversal and provide us with the `label` and `Pop` info for the labels that are needed in that traversal.
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "vkagamlyk (via GitHub)" <gi...@apache.org>.
vkagamlyk commented on PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#issuecomment-2869052494
Hi @vaibhavm99, will this change allow to save not the whole Path, but only the parts that are actually used?
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#issuecomment-2868041810
## [Codecov](https://app.codecov.io/gh/apache/tinkerpop/pull/3118?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `88.07339%` with `13 lines` in your changes missing coverage. Please review.
> Project coverage is 78.20%. Comparing base [(`cfd6889`)](https://app.codecov.io/gh/apache/tinkerpop/commit/cfd68897ea111e3ae7100dbc2f0f7a364fd0ffa5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`b5a2da8`)](https://app.codecov.io/gh/apache/tinkerpop/commit/b5a2da85bdf68d23f0432bbbe1a0159e48094c33?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 240 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/tinkerpop/pull/3118?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...rocess/traversal/step/map/TraversalSelectStep.java](https://app.codecov.io/gh/apache/tinkerpop/pull/3118?src=pr&el=tree&filepath=gremlin-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Ftinkerpop%2Fgremlin%2Fprocess%2Ftraversal%2Fstep%2Fmap%2FTraversalSelectStep.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JlbWxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9wcm9jZXNzL3RyYXZlcnNhbC9zdGVwL21hcC9UcmF2ZXJzYWxTZWxlY3RTdGVwLmphdmE=) | 0.00% | [9 Missing :warning: ](https://app.codecov.io/gh/apache/tinkerpop/pull/3118?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...kerpop/gremlin/process/traversal/step/Scoping.java](https://app.codecov.io/gh/apache/tinkerpop/pull/3118?src=pr&el=tree&filepath=gremlin-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Ftinkerpop%2Fgremlin%2Fprocess%2Ftraversal%2Fstep%2FScoping.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JlbWxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9wcm9jZXNzL3RyYXZlcnNhbC9zdGVwL1Njb3BpbmcuamF2YQ==) | 71.42% | [2 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/tinkerpop/pull/3118?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #3118 +/- ##
============================================
+ Coverage 77.87% 78.20% +0.33%
- Complexity 13578 13800 +222
============================================
Files 1015 1024 +9
Lines 59308 60147 +839
Branches 6835 7022 +187
============================================
+ Hits 46184 47040 +856
+ Misses 10817 10752 -65
- Partials 2307 2355 +48
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/tinkerpop/pull/3118?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#issuecomment-2877340952
I see this PR is currently targeting the master branch, this branch is currently accumulating changes for TinkerPop 4.0.0-beta.2, and ultimately TinkerPop 4. I would recommend targeting this change to `3.7-dev`, which is accumulating changes for 3.7.4 (non-breaking only), or `3.8-dev` which is for TinkerPop 3.8.0 (breaking changes permitted). Any changes introduced to an older development branch will also get introduced to the newer branches when the PR is merged.
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on code in PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#discussion_r2135828354
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/Scoping.java:
##########
@@ -166,6 +167,23 @@ public default <S> S getNullableScopeValue(final Pop pop, final String key, fina
*/
public Set<String> getScopeKeys();
+ /**
+ * Used to get PopInstruction of a Step that implements Scoping. PopInstruction includes the labels it needs, and the
+ * pop type for each label.
+ *
+ * @return A Set of {@link org.apache.tinkerpop.gremlin.process.traversal.step.PopContaining.PopInstruction} values which contain the label and Pop value
Review Comment:
nit: TinkerPop code format general prefers this reference being imported than being long form in the javadoc.
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on code in PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#discussion_r2135660038
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/PopContaining.java:
##########
@@ -0,0 +1,104 @@
+/*
+ * 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;
+
+import java.util.HashSet;
+import java.util.Objects;
+
+import org.apache.tinkerpop.gremlin.process.traversal.Pop;
+
+/**
+ * The {@code PopContaining} interface is implemented by traversal steps that maintain Pop instructions
+ * for label access. It provides a mechanism to track and manage how labeled elements should
+ * be accessed using {@link Pop} semantics (first, last, all, or mixed).
+ *
+ * In Gremlin traversals, various elements can be labeled and later accessed via these labels.
+ * The {@link Pop} enum determines how to access these labeled elements when there are multiple
+ * values associated with the same label.
+ *
+ * <pre>
+ * {@code
+ * // Simple example with default Pop.last behavior
+ * gremlin> g.V().as("a").out().as("a").select("a")
+ * ==>[v[2]] // returns the last element labeled "a"
+ *
+ * // Using Pop.first to get the first labeled element
+ * gremlin> g.V().as("a").out().as("a").select(first, "a")
+ * ==>[v[1]] // returns the first element labeled "a"
+ *
+ * // Using Pop.all to get all labeled elements
+ * gremlin> g.V().as("a").out().as("a").select(all, "a")
+ * ==>[v[1], v[2]] // returns all elements labeled "a"
+ * }
+ * </pre>
+ *
+ * Steps implementing this interface maintain a collection of {@link PopInstruction} objects, each containing
+ * a label and a {@link Pop} value that specifies how to access elements with that label.
+ *
+ * @author Vaibhav Malhotra
+ */
+public interface PopContaining {
+ public HashSet<PopInstruction> getPopInstructions();
+ /**
+ * A class for storing the Scope Context. It has two elements:
+ * - label: String
+ * - pop: Pop value
+ */
+ class PopInstruction {
+ public Pop pop;
Review Comment:
shouldn't these be `private` access member variables?
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on code in PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#discussion_r2087349586
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/Scoping.java:
##########
@@ -166,6 +167,13 @@ public default <S> S getNullableScopeValue(final Pop pop, final String key, fina
*/
public Set<String> getScopeKeys();
+ /**
+ * Used to get ScopingInfo of a Step that implements Scoping. Scoping info includes the labels it needs, and the pop type for each label.
+ *
+ * @return A Set of {@link ScopingInfo} values which contain the label and Pop value
+ */
+ public default Set<ScopingInfo> getScopingInfo() {return null; }
Review Comment:
Should there be a default implementation here? If something implements `Scoping` shouldn't it have a proper implementation? or are there cases where you would return `getScopeKeys` but not `getScopingInfo`? and if so, would `return null` be the right default?
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on code in PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#discussion_r2087347460
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/Scoping.java:
##########
@@ -185,4 +193,39 @@ public Scoping getStep() {
return step;
}
}
+
+
+ /**
+ * A class for storing the Scoping information. It has two elements:
+ * - label: String
+ * - pop: Pop value
+ */
+ public static class ScopingInfo {
Review Comment:
Just a comment on naming - we have `getScopeKeys` so I feel like we should leave the "ing" suffix for the interface and the non-active "Scope" for the things the interface exposes. Perhaps we call this `ScopeInfo` or `ScopeContext` (I prefer the latter i think because we don't use "Info" anywhere in our code, but we do use "Context" in places) and have a `Scoping.getScopeContext()`?
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on code in PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#discussion_r2135840793
##########
gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectOneStepTest.java:
##########
@@ -61,4 +64,57 @@ public void shouldRequirePathsAccordingly() {
assertEquals(traversalPath[0], ((Traversal.Admin<?, ?>) traversalPath[1]).getTraverserRequirements().contains(TraverserRequirement.LABELED_PATH));
}
}
+
+ @Test
+ public void testPopInstruction() {
Review Comment:
nit: note that TinkerPop tests prefer "should" prefix over "test" for test name, like "shouldObtainPopInstructions" would be a good name here. please take a look at other tests in the PR and make changes accordingly.
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on code in PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#discussion_r2135830890
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/PopContaining.java:
##########
@@ -0,0 +1,104 @@
+/*
+ * 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;
+
+import java.util.HashSet;
+import java.util.Objects;
+
+import org.apache.tinkerpop.gremlin.process.traversal.Pop;
+
+/**
+ * The {@code PopContaining} interface is implemented by traversal steps that maintain Pop instructions
+ * for label access. It provides a mechanism to track and manage how labeled elements should
+ * be accessed using {@link Pop} semantics (first, last, all, or mixed).
+ *
+ * In Gremlin traversals, various elements can be labeled and later accessed via these labels.
+ * The {@link Pop} enum determines how to access these labeled elements when there are multiple
+ * values associated with the same label.
+ *
+ * <pre>
+ * {@code
+ * // Simple example with default Pop.last behavior
+ * gremlin> g.V().as("a").out().as("a").select("a")
+ * ==>[v[2]] // returns the last element labeled "a"
+ *
+ * // Using Pop.first to get the first labeled element
+ * gremlin> g.V().as("a").out().as("a").select(first, "a")
+ * ==>[v[1]] // returns the first element labeled "a"
+ *
+ * // Using Pop.all to get all labeled elements
+ * gremlin> g.V().as("a").out().as("a").select(all, "a")
+ * ==>[v[1], v[2]] // returns all elements labeled "a"
+ * }
+ * </pre>
+ *
+ * Steps implementing this interface maintain a collection of {@link PopInstruction} objects, each containing
+ * a label and a {@link Pop} value that specifies how to access elements with that label.
+ *
+ * @author Vaibhav Malhotra
+ */
+public interface PopContaining {
+ public HashSet<PopInstruction> getPopInstructions();
Review Comment:
do we explicitly need to use the implementation `HashSet` here? if not, I would think `Set` is preferred.
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "xiazcy (via GitHub)" <gi...@apache.org>.
xiazcy commented on PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#issuecomment-2906001670
LGTM, thanks for the change! 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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "vaibhavm99 (via GitHub)" <gi...@apache.org>.
vaibhavm99 commented on PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#issuecomment-2874886826
> Hi @vaibhavm99, thanks for opening a PR! I should have time to give it a full review by Monday at the latest. Could I ask a bit about the motivation for the new `getScopingInfo()` method? The label functionality is mostly covered by `getScopeKeys()`, is the main objective to give some common interface for finding the Pop value of "select" type steps? Is this change in response to any difficulties with the current interface?
`getScopeKeys` only gives us a List of labels needed, but does not contain the `Pop` info. This is a common interface to get both `label` and the `Pop` value for all the labels needed. This is to provide label information inside the repeat step during the Neptune FastTranslation.
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#issuecomment-2968612475
Thanks for coming through and making all of these changes @vaibhavm99. My vote is VOTE +1.
Note that I intend to cherry pick this back to 3.7 when merging as this is completely non-breaking and can probably deliver some value sooner from that branch.
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer merged PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on code in PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#discussion_r2144051119
##########
gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/TestDataBuilder.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * 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;
+
+import java.util.HashSet;
+
+import org.apache.tinkerpop.gremlin.process.traversal.Pop;
+import org.apache.tinkerpop.gremlin.process.traversal.step.PopContaining;
+
+/**
+ * This class is responsible for building test data for `PopInstruction` Unit tests.
+ * It provides methods to create a set of `PopInstruction` test data objects.
+ *
+ */
+public class TestDataBuilder {
+
+ // Helper function to create a Set of `PopInstruction` values
+ public static HashSet<PopContaining.PopInstruction> createPopInstructionSet(Object[]... pairs) {
Review Comment:
```suggestion
public static HashSet<PopContaining.PopInstruction> createPopInstructionSet(final Object[]... pairs) {
```
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/PopContaining.java:
##########
@@ -0,0 +1,103 @@
+/*
+ * 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;
+
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.tinkerpop.gremlin.process.traversal.Pop;
+
+/**
+ * The {@code PopContaining} interface is implemented by traversal steps that maintain Pop instructions
+ * for label access. It provides a mechanism to track and manage how labeled elements should
+ * be accessed using {@link Pop} semantics (first, last, all, or mixed).
+ *
+ * In Gremlin traversals, various elements can be labeled and later accessed via these labels.
+ * The {@link Pop} enum determines how to access these labeled elements when there are multiple
+ * values associated with the same label.
+ *
+ * <pre>
+ * {@code
+ * // Simple example with default Pop.last behavior
+ * gremlin> g.V().as("a").out().as("a").select("a")
+ * ==>[v[2]] // returns the last element labeled "a"
+ *
+ * // Using Pop.first to get the first labeled element
+ * gremlin> g.V().as("a").out().as("a").select(first, "a")
+ * ==>[v[1]] // returns the first element labeled "a"
+ *
+ * // Using Pop.all to get all labeled elements
+ * gremlin> g.V().as("a").out().as("a").select(all, "a")
+ * ==>[v[1], v[2]] // returns all elements labeled "a"
+ * }
+ * </pre>
+ *
+ * Steps implementing this interface maintain a collection of {@link PopInstruction} objects, each containing
+ * a label and a {@link Pop} value that specifies how to access elements with that label.
+ *
+ */
+public interface PopContaining {
+ public Set<PopInstruction> getPopInstructions();
+ /**
+ * A class for storing the Scope Context. It has two elements:
+ * - label: String
+ * - pop: Pop value
+ */
+ class PopInstruction {
+ private final Pop pop;
+ private final String label;
+
+ public PopInstruction(String label, Pop pop) {
+ this.pop = pop;
+ this.label = label;
+ }
+
+ public PopInstruction(Pop pop, String label) {
Review Comment:
```suggestion
public PopInstruction(final Pop pop, final String label) {
```
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on code in PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#discussion_r2135653400
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/PopContaining.java:
##########
@@ -0,0 +1,104 @@
+/*
+ * 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;
+
+import java.util.HashSet;
+import java.util.Objects;
+
+import org.apache.tinkerpop.gremlin.process.traversal.Pop;
+
+/**
+ * The {@code PopContaining} interface is implemented by traversal steps that maintain Pop instructions
+ * for label access. It provides a mechanism to track and manage how labeled elements should
+ * be accessed using {@link Pop} semantics (first, last, all, or mixed).
+ *
+ * In Gremlin traversals, various elements can be labeled and later accessed via these labels.
+ * The {@link Pop} enum determines how to access these labeled elements when there are multiple
+ * values associated with the same label.
+ *
+ * <pre>
+ * {@code
+ * // Simple example with default Pop.last behavior
+ * gremlin> g.V().as("a").out().as("a").select("a")
+ * ==>[v[2]] // returns the last element labeled "a"
+ *
+ * // Using Pop.first to get the first labeled element
+ * gremlin> g.V().as("a").out().as("a").select(first, "a")
+ * ==>[v[1]] // returns the first element labeled "a"
+ *
+ * // Using Pop.all to get all labeled elements
+ * gremlin> g.V().as("a").out().as("a").select(all, "a")
+ * ==>[v[1], v[2]] // returns all elements labeled "a"
+ * }
+ * </pre>
+ *
+ * Steps implementing this interface maintain a collection of {@link PopInstruction} objects, each containing
+ * a label and a {@link Pop} value that specifies how to access elements with that label.
+ *
+ * @author Vaibhav Malhotra
Review Comment:
please don't "author" tags - any author tags currently in the code base are there mostly for legacy reasons. going forward new work doesn't include them. git is the best source of authorship.
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on code in PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#discussion_r2135838054
##########
gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/TraversalSelectStepTest.java:
##########
@@ -0,0 +1,35 @@
+package org.apache.tinkerpop.gremlin.process.traversal.step.map;
+
+import java.util.HashSet;
+
+import org.apache.tinkerpop.gremlin.TestDataBuilder;
+import org.apache.tinkerpop.gremlin.process.traversal.Pop;
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
+import org.apache.tinkerpop.gremlin.process.traversal.step.PopContaining;
+
+import junit.framework.TestCase;
+
+public class TraversalSelectStepTest extends TestCase {
Review Comment:
please prefer JUnit4 syntax with `@Test` annotation
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#issuecomment-2868038073
Hi @vaibhavm99, thanks for opening a PR! I should have time to give it a full review by Monday at the latest. Could I ask a bit about the motivation for the new `getScopingInfo()` method? The label functionality is mostly covered by `getScopeKeys()`, is the main objective to give some common interface for finding the Pop value of "select" type steps? Is this change in response to any difficulties with the current interface?
--
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
Re: [PR] getScopingInfo utility added [tinkerpop]
Posted by "vkagamlyk (via GitHub)" <gi...@apache.org>.
vkagamlyk commented on PR #3118:
URL: https://github.com/apache/tinkerpop/pull/3118#issuecomment-2875019246
> > Hi @vaibhavm99, will this change allow to save not the whole Path, but only the parts that are actually used?
>
> Hi @vkagamlyk , I'm sorry I don't understand what you mean by "the whole Path." This change should not have any breaking problems with older code, as it is just adding a new API. This would take in a traversal and provide us with the `label` and `Pop` info for the labels that are needed in that traversal.
Hi @vaibhavm99!
In some cases it is necessary to transfer Traversers between servers. Part of Traverser can be Path object that is larger than other Traverser parts. If we know in advance which parts of the path are used, then we can transfer only them.
Path is list of pairs <label, value>, `getScopeKeys` can be used to get list of used labels.
--
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