Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Remove “in” expression workaround - #183

Merged
1ec5 merged 3 commits into
masterfrom
1ec5-expr-in-168
Mar 17, 2020
Merged

1ec5 merged 3 commits into
masterfrom
1ec5-expr-in-168

Conversation

@1ec5

@1ec5 1ec5 commented Feb 27, 2020

Copy link
Copy Markdown
Contributor

Removed the match-based workaround for in expressions in favor of real support for an in expression operator, including support for testing whether a string is a substring of another string.

  • Replace match-based workaround with in operator mappings
  • Test substring matching
  • Implement ANY/SOME and NONE with = and ALL with !=
  • Document in expressions in “Information for Style Authors” guide
  • Document comparison modifiers in “Predicates and Expressions” guide

Fixes #168. Depends on #181. Retarget and rebase this PR onto master before merging.

/cc @mapbox/maps-ios

@1ec5 1ec5 added enhancement New feature or request expressions labels Feb 27, 2020
@1ec5 1ec5 added this to the release-vanillashake milestone Feb 27, 2020
@1ec5
1ec5 requested review from a team and fabian-guerra February 27, 2020 15:36
@1ec5 1ec5 self-assigned this Feb 27, 2020
@1ec5

1ec5 commented Feb 28, 2020

Copy link
Copy Markdown
Contributor Author

I threw in some additional support for NSExpression syntactic sugar so developers don’t feel like they have to match our examples exactly:

  • ANY x = y is logically equivalent to x CONTAINS y and y IN x. All three forms evaluate to an in expression.
  • ALL x != y is logically equivalent to NOT x CONTAINS y and NOT y IN x. All three forms evaluate to an in expression inside a ! expression.

@1ec5 1ec5 removed the 🛑 blocked label Feb 28, 2020
@1ec5
1ec5 requested a review from jmkiley February 29, 2020 02:56
@1ec5
1ec5 changed the base branch from 1ec5-gl-v1.2.0 to master February 29, 2020 07:14
@jmkiley

jmkiley commented Mar 3, 2020

Copy link
Copy Markdown
Contributor

I tested this PR out with point features with a title attribute. title had a string containing feature for all three, but I wasn't able to get any features with [NSPredicate predicateWithFormat:@"title CONTAINS 'feature'"]; Also tested with IN and didn't have any luck.

Sample code
MGLPointFeature *pointfeature1 = [[MGLPointFeature alloc] init];
pointfeature1.coordinate = CLLocationCoordinate2DMake(0, 33);
[pointfeature1 setAttributes:@{@"title": [NSString stringWithFormat:@"feature-%f", pointfeature1.coordinate.longitude]}];

MGLPointFeature *pointfeature2 = [[MGLPointFeature alloc] init];
pointfeature2.coordinate = CLLocationCoordinate2DMake(0, 66);
[pointfeature2 setAttributes:@{@"title": [NSString stringWithFormat:@"feature"]}];

MGLPointFeature *pointfeature3 = [[MGLPointFeature alloc] init];
pointfeature3.coordinate = CLLocationCoordinate2DMake(0, 99);
[pointfeature3 setAttributes:@{@"title": [NSString stringWithFormat:@"feature"]}];
                                           
NSArray *points = @[pointfeature1, pointfeature2, pointfeature3];
MGLShapeSource *source = [[MGLShapeSource alloc] initWithIdentifier:@"source" features:points options:nil];

[style addSource:source];

MGLCircleStyleLayer *layer = [[MGLCircleStyleLayer alloc] initWithIdentifier:@"layer" source:source];
layer.predicate = [NSPredicate predicateWithFormat:@"title CONTAINS 'feature'"];
layer.circleRadius = [NSExpression expressionForConstantValue:@40];
[style addLayer:layer];

@jmkiley

jmkiley commented Mar 3, 2020

Copy link
Copy Markdown
Contributor

Noting that CONTAINS seems to work in combination with the NOT operator. Two circles displayed with [NSPredicate predicateWithFormat:@"NOT title CONTAINS '33'"];.

Also, using CONTAINS with an NSExpression works. For example, I saw one circle with layer.circleOpacity = [NSExpression expressionWithFormat:@"TERNARY(title CONTAINS '33', 1, 0)"];.

@1ec5

1ec5 commented Mar 3, 2020

Copy link
Copy Markdown
Contributor Author

I tested this PR out with point features with a title attribute. title had a string containing feature for all three, but I wasn't able to get any features with [NSPredicate predicateWithFormat:@"title CONTAINS 'feature'"];

Thanks for noticing that: mapbox/mapbox-gl-native#16262.

@fabian-guerra fabian-guerra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Using match for in expressions what kind of problematic. With this change it became simpler.


return matchExpression.mgl_jsonExpressionObject;
op = @"in";
break;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏🏼 this is simple

@1ec5
1ec5 force-pushed the 1ec5-expr-in-168 branch from 12903be to d7058e7 Compare March 11, 2020 23:05
@1ec5

1ec5 commented Mar 13, 2020

Copy link
Copy Markdown
Contributor Author

I tested this PR out with point features with a title attribute. title had a string containing feature for all three, but I wasn't able to get any features with [NSPredicate predicateWithFormat:@"title CONTAINS 'feature'"];

Thanks for noticing that: mapbox/mapbox-gl-native#16262.

in filters have been fixed in mbgl, and #210 will pull in that fix. No additional changes will be needed at the SDK level to support IN in predicates, but I’d like to add unit tests that round-trip between NSPredicate and mbgl filters, which will depend on #210.

@1ec5
1ec5 force-pushed the 1ec5-expr-in-168 branch from d7058e7 to 887f10e Compare March 17, 2020 02:40
1ec5 added 3 commits March 17, 2020 01:46
Removed the match-based in expression workaround in favor of real support for an in expression operator. Avoid interpreting “literal IN literal” predicates as legacy filters.
Added support for “ANY … =” and “ALL … !=” as synonyms for “IN” and “NOT IN”, respectively.
@1ec5
1ec5 force-pushed the 1ec5-expr-in-168 branch from 887f10e to 342606d Compare March 17, 2020 08:48
@1ec5
1ec5 merged commit 342606d into master Mar 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request expressions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add expression "in"

4 participants