-
-
Notifications
You must be signed in to change notification settings - Fork 68
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
Design Proposal: Allow creating arrays with expressions #950
Comments
Relevant years-old mapbox-gl-js feature request: mapbox/mapbox-gl-js#6155 with folks still asking for this functionality to this day |
This touches on a more fundamental issue. The open-ended arrays are reasonably idiomatic for these properties in JavaScript and JSON, but when translating the runtime styling API to Objective-C/Swift, it became necessary to represent these same properties as something more strongly typed, typically a type that’s built into the standard library, such as The JSON format takes this same approach with a more limited set of types, such as colors. Maybe there need to be more type constructing operators like More facilities for working with arrays will be useful for all the platforms in these cases regardless, but I suspect the NSExpression representation on iOS would need to be more lenient about arrays versus these more specific types. |
Similarly, MapLibre Compose uses the Compose If we had say, |
Based on the proposed operator names An observation: there’s only one |
If you use one function name for both, how would you determine the output format? Objects need keys and values, while an array just needs a sequence of elements. Two signatures implies two functions. Also curious if you're aware of any use cases for objects here; I'm only aware of array use cases. Literal wraps a single object or array, and doesn't allow function calls inside. That's the reason it can support both input types with one name. I don't have a string opinion on the Java/Kotlin/ObjC/Swift name, but there's already precedent for other expression functions with symbols as names. On Java/Kotlin I imagine it would be a function taking vararg Expression, perhaps Considering naming, I think the constraints on the different platforms are different (ability to define reusable functions, ability to add overloads, etc) so I feel that naming things exactly the same on all platforms is an anti goal. Happy to defer on that though; I really only care about the functionality. On the MapLibre Compose side, we already use different, Kotlin idiomatic names for all the expressions. We have type safe functions to create various expression types and would probably add a overloads accepting expressions of particular types instead of the corresponding primitives we currently accept. |
That all sounds reasonable to me. I was just spitballing about a way to avoid the punctuation if desired, but I agree that there’s no hard constraint about keeping the operator name consistent across platforms. (Otherwise, I would be more guilty than anyone for having violated that constraint over the years.) Wrapping both arrays and objects with the same operator probably won’t work for the reason you pointed out. There might be a use case for wrapping an object. For example, if the tiles have a feature property that indicates the language code of the text in another feature property, such as |
I think if this proposal is accepted, and if I'd be happy to make a separate proposal to discuss the parameter scheme for the |
Can you better clarify why |
Consider Sure, there's probably not a style with nested literal arrays of strings, because nothing in the style spec requires that. But consider Interestingly, I discovered recently MapLibre Native rejects nested arrays inside literal via some code paths but not others. So my impression is perhaps literal was meant to evaluate expressions, it was never actually implemented, but now it evaluates sub arrays as literal arrays, which even though it's not fully implemented, it's implemented enough that it'd break currently valid styles if literal begins to evaluate expressions inside. Personally, I think it's valid to have both (LISP like languages often do) but once |
See: quote and quasiquote in Lisp: https://courses.cs.washington.edu/courses/cse341/04wi/lectures/14-scheme-quote.html
Actually, after sleeping on it, I think it's possible if the syntax is like literal (one parameter, either object or array) with the only difference being arrays in elements/values are evaluated as expressions. If we take that approach, I'd recommend Not 100% sure how that'd work with the parser, but I can dig into it. If maintainers agree that something with this functionality is likely desirable, I can start hacking on a PR to experiment with the implementation. |
The original reason for |
That's super interesting context. How did other array properties work back before literal (did they exist)? Was But yeah, the code that does static analysis of possible outputs for those properties would need to be updated to support the proposed |
This would’ve been around the time that expressions were being implemented in the first place. In the previous style function syntax, which didn’t have arbitrary arrays, properties like |
It's still not clear to me why literal isn't good enough. The following example is using a "reserved word" - get, so there's a need to evaluate the content before assigning it to an array:
I'm still confused why there's a need to create a new operator (not to mention the confusion other users will have when they'd need to understand which one to use). As someone who reads these expressions I'm usually able to understand what the intention was, I would expect the parser to be able to do the same... |
Because array syntax is indistinguishable from function syntax, a choice needs to be made on which to assume. And literal already made that choice one way; changing it to the other way would be a breaking change. In my use case, I want to do some math to calculate the x and y components of an offset. That requires function calls inside an array. Function calls are themselves represented by arrays in the JSON syntax. So an array with x and y calculated by functions means an array with sub arrays. The existing behavior of literal is to treat sub arrays themselves as literal arrays, not function calls. We could change that, but existing styles with sub arrays inside literal would be parsed differently. Or, an example:
Does that output an array of two numbers calculated by multiplication? Or does that output an array of two sub arrays, each with a string and two numbers? Looking at it as a human, I can tell the intent of the writer is the first. But looking at it as a parser, the existing behavior today is the second. If maintainers are cool with a breaking change, I can make that PR. But I assume we'd prefer to avoid a breaking change here. |
I think literal behavior is broken and we should fix it. |
If I set |
If we change parsing to treat arrays starting with "reserved words" differently from other arrays, and every function name is a reserved word, then every new function added to the spec becomes a breaking change. If that's acceptable I'm happy to PR it, but I'd advise against it. The literal operator is fundamentally the same thing as quote in Lisp languages, so solving it the same way (quasiquote operator) doesn't seem that bad to me. Call it |
I guess I lack the relevant knowledge to understand what I'm missing. Maybe someone else can approve this then... |
I filed a PR so we have something more concrete to hack on: #951 naming is tbd (currently |
I would suggest to present this in the next monthly meeting and discuss the pros and cons on each approach. |
I usually have a conflict at the time of the monthly meeting, so can't usually attend. Will see if I can rearrange things on the day of the next meeting. |
Design Proposal: Allow creating arrays with expressions
Motivation
A number of layer properties take arrays as their values. For example, see
text-offset
andtext-variable-anchor-offset
. The only way to produce an array in the existing expression system is to either get it from a feature withget
, or create it withliteral
. Neither of these allow calculating the elements of the array with math expressions, though you can select between different literal arrays or properties with conditionalmatch
/case
expressions.For arrays of just numbers (let's call them vectors), one can work around this for certain use cases by interpolating between multiple vectors, and doing your math on the interpolation input. MapLibre Compose does this workaround to simulate multiplying the magnitude of a
text-offset
(to convert between SP and EM units).For arrays containing other types, not even that workaround is available (example:
text-variable-anchor-offset
). MapLibre Compose was unable to work around this.Some use cases that could benefit from expressions in array elements:
sin
andcos
, such as to offset in the direction of some angle (see this StackOverflow question: https://stackoverflow.com/questions/52715411/mapbox-gl-data-driven-styling-for-text-offset)text-max-width
bytext-size
to produce EMtext-max-width
), so the multiplication must be done in expression evaluation, not pre processing. This works great for single number properties but runs into problems with array properties (text-offset
andtext-variable-anchor-offset
).Proposed Change
I propose a new function in the style spec,
[]
, that takes any number of expression arguments and returns an array of all those arguments. The syntax would look like:This is in contrast to
literal
, which does not evaluate its arguments as expressions.API Modifications
Just the new function
[]
. Also perhaps a similar function{}
for objects.Migration Plan and Compatibility
No migration needed for the main proposal; this is new functionality.
Some alternatives in the "Rejected Alternatives" section below are backwards incompatible and would need a migration if they're selected.
Rejected Alternatives
literal
to evaluate its elements as expressions: this would not be backwards compatible in cases of nested arrays. The only place in the style spec that uses nested arrays istext-variable-anchor-offset
, whose value looks like["top", [1, 2], "bottom", [3, 4]]
. If it weren't for that backwards incompatibility, I'd prefer this alternative.literal
to evaluate its elements as expressions: I think this is confusing to the reader, as it's not immediately apparent whattrue
orfalse
mean when reading a style spec.Also, my impression (haven't read the code so perhaps wrong) is thatliteral
is part of the expression parser, and not part of expression evaluation like other functions.concat
. Feels inconsistent.[[ ["get", "x"], ["get", "y"] ]]
: Syntax is ambiguous in case of single element arrays where the first element is an array of strings (looks like a function call). Would need some sort of escape like the above alternative.[]
:collator
that creates a Collator,format
that creates a Formatted , andimage
that creates a ResolvedImage. So, the natural convention might bearray
to create an Array, but that function name is already taken for a function to assert that the input is an array (similar tostring
,number
,boolean
, andobject
)to-color
,to-string
, etc, which all take a single value and convert it to the target type (some with fallbacks). The nameto-array
is available, but the convention doesn’t quite match what we want.rgb
andrgba
are closest in concept to what we want (evaluate some arguments as expressions, construct a result object from it) but of course that naming convention doesn't have an obvious analogue for arbitrary arrays.new-array
is pretty clear in intent, but could get visually noisy in case of nested arrays. I don't really dislike this, just like[]
a bit better as it would take less of the reader's focus away from the content of the array.[]
.EDIT: additional alternatives as considered in the issue thread
point
andpadding
specifically for those types: Solves the issue for some properties (text-offset
) but not others (text-variable-anchor-offset
,line-dasharray
, etc). I think it's worthwhile to have those but we'd still need[]
in addition to that.The text was updated successfully, but these errors were encountered: