Implement none() Filter Step
#2352
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## 3.7-dev #2352 +/- ##
=============================================
- Coverage 76.22% 76.18% -0.04%
- Complexity 13117 13128 +11
=============================================
Files 1082 1084 +2
Lines 64967 65024 +57
Branches 7252 7264 +12
=============================================
+ Hits 49521 49540 +19
- Misses 12752 12791 +39
+ Partials 2694 2693 -1 ☔ View full report in Codecov by Sentry. |
| @@ -2008,7 +2009,7 @@ public default GraphTraversal<S, E> filter(final Traversal<?, ?> filterTraversal | |||
| * | |||
| * @return the updated traversal with respective {@link NoneStep}. | |||
| */ | |||
| @Override | |||
There was a problem hiding this comment.
Nit: You can probably leave this override annotation in there for now.
| @@ -4078,6 +4092,7 @@ private Symbols() { | |||
| public static final String dateDiff = "dateDiff"; | |||
| public static final String all = "all"; | |||
| public static final String any = "any"; | |||
| public static final String listNone = "none"; | |||
There was a problem hiding this comment.
Nit: Could you have just called dthis none instead of listNone?
There was a problem hiding this comment.
The "none" token is already on Traversal.Symbols - shouldn't that just be used?
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/__.java
Show resolved
Hide resolved
| g.inject(xx1).none(TextP.startingWith("a")) | ||
| """ | ||
| When iterated to list | ||
| Then the result should be empty |
There was a problem hiding this comment.
I'm not sure this is the right result because
gremlin> g.inject([null,"bcd"]).all(TextP.startingWith("a"))
gremlin>
|
Missing provider semantics documentation and you could probably add an update to the upgrade documentation as well. |
| @@ -2008,7 +2009,7 @@ public default GraphTraversal<S, E> filter(final Traversal<?, ?> filterTraversal | |||
| * | |||
| * @return the updated traversal with respective {@link NoneStep}. | |||
| */ | |||
| @Override | |||
| @Deprecated | |||
There was a problem hiding this comment.
This deprecation needs javadoc, especially in this case. Here's an example of the form:
| @@ -40,7 +40,7 @@ This release also includes changes from <<release-3-6-6, 3.6.6>> and <<release-3 | |||
| * Added new data type `DT` to represent periods of time. | |||
| * Added Gherkin support for Date. | |||
| * Extended `datetime()` function to produce a current server date. | |||
| * Added list filtering functions `all` and `any`. | |||
| * Added list filtering functions `all`, `any`, and `none`. | |||
There was a problem hiding this comment.
there is a major deprecation to document here. there should also be provider upgrade docs as none() is an important part of server interactions.
| */ | ||
| public default <S2> GraphTraversal<S, E> none(final P<S2> predicate) { | ||
| this.asAdmin().getBytecode().addStep(Symbols.listNone, predicate); | ||
| return this.asAdmin().addStep(new ListNoneStep<>(this.asAdmin(), predicate)); |
There was a problem hiding this comment.
I realize NoneStep is already taken and has special meaning that is not easily removed at this moment. Given that none() is just not(any()) and the code for this ListNoneStep and AnyStep, i think it would be better to just give AnyStep a negation configuration. Then you get rid of ListNoneStep.
| @@ -2309,6 +2309,25 @@ 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#limit(org.apache.tinkerpop.gremlin.process.traversal.Scope,long)++[`limit(Scope,long)`] | |||
| link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/Scope.html++[`Scope`] | |||
|
|
|||
| [[list-none-step]] | |||
| === List None Step | |||
There was a problem hiding this comment.
Users will still see this as none() so I think it's better to explain the two usages in the existing section for that step. Luckily none() is mostly an internal step. it's unlikely anyone is really using it directly.
ryn5
changed the title
none() Filter StepTINKERPOP-2978 Implement all() and any() steps. none() Filter Step
Takes
lists,predicateand filters out lists which have at least 1 element matching the predicate. Changes based on existingall()andany()list functions in TINKERPOP-2978 Implement all() and any() steps..