-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
draft of expressions examples doc #4836
Conversation
* Add draft proposal for property expressions spec Developed in #4715 * Update spec proposal * Introduce `curve` expression, use it to cover zoom function `stops`. * Introduce object and array literals * Remove explicitly typed property lookup * Describe type system * Remove ["else"] * Minor edits
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @nickidlugash! These are super helpful -- some comments inline.
"icon-image": { | ||
"expression" : [ | ||
"concat",["string", ["get", ["properties"], "shield"]], "-", ["string", ["get", ["properties"], "ref"]] | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the after the "-"
supposed to be wrapped in length
, like ["length", ["string", ["get", ["properties"], "ref"]]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops, no – that property should be reflen
, which is the same thing as ^, pre-calculated in the data, and with the exception that values of 1
become 2
(because we use the same icon for both). I don't know yet if we would consider dropping this property in favor of using length
(since we have users of our tiles that don't use Mapbox GL), but I'll try rewriting this expression to use length
anyway, as an example.
"city-dot-small" | ||
] | ||
] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could use a "step"
curve here as well:
[
"curve", ["step"],
0, ["case", ["==", ["number", ["get", ["properties"], "capital"]], 2], "capital-city-dot-large", "city-dot-large" ],
2, ["case", ["==", ["number", ["get", ["properties"], "capital"]], 2], "capital-city-dot-medium", "city-dot-medium" ],
5, ["case", ["==", ["number", ["get", ["properties"], "capital"]], 2], "capital-city-dot-small", "city-dot-small" ]
]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, except for one thing: as of now, step
curves (like their predecessor, interval
functions) use intervals that are inclusive of the left-hand boundary and exclusive of the right-hand one. We could make this configurable (["step", ">"]
/ ["step", ">="]
), or we could just rely on a workaround like using 2.001
instead of 2
as the step value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for this example (since the values are integers), I think the steps: 0, 3, 6 would be the same as the current case
expression?
["<=", ["number", ["get", ["properties"], "scalerank"]], 5], | ||
9, | ||
[">", ["number", ["get", ["properties"], "scalerank"]], 5], | ||
6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, this case
could be a step
curve instead.
"expression": [ | ||
"curve", "step", ["zoom"], | ||
7, | ||
["match", ["get", ["properties"], "ldir"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the ["get", ...
here should be wrapped in a ["string", ...
"S", "center", | ||
"SW", "right", | ||
"W", "right", | ||
"NW", "right" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs a final output value in case ldir
doesn't match one of these values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anandthakker ah, just realizing now that y_else
is specified for match
. Should I add a final output value to all the match
expressions in this file? Is it an invalid expression if the final output value is not included?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yah, it'll fail validation if not included
Thanks for these examples @nickidlugash -- very instructive to look at some real use cases. The first thing that jumps out at me is that while our current syntax enables a lot of flexibility, it leave a lot to be desired in terms of ergonomics and concision. It's never too early to begin thinking about how we could improve this, so here are the things that come to my mind:
|
22, | ||
[">", ["number", ["get", ["properties"], "scalerank"]], 5], | ||
22 | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anandthakker Can I drop this second case expression and just make this value: 14, 22
? Will the renderers be able to calculate an exponential curve like this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah hah -- yep! that should work!
…"case" expressions with comparisons to "step curves", adding final output values for all "match" expressions, adding missing type conversion, fixing highway shield icon-image example to use "length"
@anandthakker @jfirebaugh what are the other available options besides
I anticipate that this will be a fairly commonly used syntax pattern for our core styles. I'm happy to keep these examples updated as we make any improvements to the syntax. (@anandthakker up to you if you want to merge this in once all the expressions are valid/optimal according to the latest syntax definitions, or if you want to keep this separate for now). This exercise has been super helpful for me for thinking through the future of our core styles as well 🌅 |
The only immediate example I know of would be if a feature property value were to be a nested object (technically possible in GeoJSON, and maybe in the future in vector tiles as well). E.g.
I wonder if it would be worthwhile to make these into integration test cases like the ones added in 63e4c64. That would let us keep checking the syntax, and also to include expected input/output values to confirm that the functions really do behave the way we think. |
Could be a good idea! Based on the master DDS-tracking ticket, some of the style properties used in this example page ( |
b75efe5
to
c1c29cb
Compare
2d82f1a
to
3bc2561
Compare
Noting that |
db7c15c
to
c37f1ea
Compare
ed3dd33
to
2c7658a
Compare
6965f91
to
9ec03f0
Compare
75fcd51
to
73fff34
Compare
d68949a
to
9ee7362
Compare
Closing; this was very useful as a design aid but I don't think we're planning to keep expressions.md around much longer. |
@anandthakker as discussed, here's a doc with some practical examples of expressions. If any of the syntax is incorrect please update 🙏
I wasn't sure where to put the file, so please move it if you wish.