Git Product home page Git Product logo

Comments (6)

jahow avatar jahow commented on June 6, 2024

Hi!

In the Maplibre style spec which is the main inspiration for the operators in OpenLayers expressions, the get operator actually allows this with a second argument: https://maplibre.org/maplibre-style-spec/expressions/#get

Note that the get operator in OL expressions takes a typeHint as 2nd param so this would also have to be handled. Type hints are string literals and not expressions so differentiating between an object and a type hint should be doable.

Would you be willing to provide a PR if that solution fits your needs? I'd be happy to provide guidance the best I can.

from openlayers.

tschaub avatar tschaub commented on June 6, 2024

I agree it makes sense to support deeply nested feature property values. I'm not sure that the nested expression is really the best way to do this.

I think the most natural / guessable syntax would be for the get operator to take multiple arguments representing the nested path.

So for a feature with properties like

{some: {deeply: {nested: {value: 42}}}}

the expression

// my ideal syntax
['get', 'some', 'deeply', 'nested', 'value']

would evaluate to 42 in the context of the feature.

If instead we followed the Mapbox style convention and accepted an optional object as the second argument to the get operator, accessing the value above would look like this:

// following mapbox
['get', 'value', ['get', 'nested', ['get', 'deeply', ['get', 'some']]]]

Both of these approaches conflict with the current signature for get that accepts a type hint as the second argument. I think it is worth revisiting that decision (and changing it in a breaking release if we agree on something else). I feel like the type hints should be replaced with type assertions like the existing number and string operators. @jahow, are there cases where a type assertion could not be used instead of a type hint?

I'm not clear on what the proposed nested operator expression would evaluate to. And I'd like to have a bit more discussion on how to best handle nested property access before committing to something.

If we want to stick close to the Mapbox style spec, then accepting an object as the optional second argument to the get operator would make sense. And if we really needed to keep the existing type hint support, we could say the second argument must either be a string literal (type hint) or an expression that evaluates to an object (like another get expression). Implementing this (with or without the type hint support) would complicate the way the get operator currently works (which is to signal which feature properties are added to the evaluation context).

The simpler alternative to implement would be the ['get', 'some', 'deeply', 'nested', 'value'] syntax. This would also be my preference for being intuitive/guessable. I know it breaks from the Mapbox convention, but we already do in other ways.

from openlayers.

jahow avatar jahow commented on June 6, 2024

Hmm, I still like the Mapbox approach quite a bit, it might be more verbose but it also makes things more explicit. This would also let your read from an array in an object using at and this sort of things. Wouldn't that be a bit awkward with just a list of string? I guess one could allow integers like so: ['get', 'array', 2, 'object', 'value'] but it feels quite implicit IMO.

About the type hint, once type assertions are available in the gpu compiler we can very likely get rid of them, yes!

from openlayers.

WouterSpaak avatar WouterSpaak commented on June 6, 2024

I agree with @tschaub that the nested proposal is a bit awkward, I figured it would be a way to minimise breaking changes to the API. It would overload get instead of changing the original call signature.

I really like the variadic API @tschaub suggests. It feels to me like very clean and intuitive API design. I'm not too keen on the verbose nature of ['get', 'parent', ['get', 'child', ['get', 'grandchild']], 'someTypeHint'] that the MapLibre spec implements, but it would be a bit easier to implement I guess, when Array.isArray(arguments[1]) we can traverse the JS object.

I'm not quite sure what the type hint argument does. Is there any documentation I could read? I'd be more than happy to help refactor something to get rid of it!

from openlayers.

jahow avatar jahow commented on June 6, 2024

The type hint is there because the get operator has no clue what type of value it will output on its own. Other operators are usually able to infer their type by various means, but the get operator isn't and in some situations, having no informating about the output type will make parsing/compiling impossible. Because a user normally knows what type a feature attribute is, providing a type hint is an easy solution this.

from openlayers.

tschaub avatar tschaub commented on June 6, 2024

The thing I'm not clear about with the type hint is where it is actually required. I understand how it works currently, but am uncertain where it is required. When parsing, we start with the top-most expression and we can know what output types are expected. For example, when parsing an expression for stroke-width, we expect a number. In the simple case of 'stroke-width': ['get', 'some-property'], we don't need a type hint. As we traverse the expressions, ideally we can narrow the types we expect. This works well if our operators are not polymorphic (for example, if + only accepts numbers and we have a separate operator like concat if we want to "add" strings together).

I know that the current implementation might need the type hints. But I'm still unclear if they are really "required" (if we change the current implementation or if we tighten up the syntax/operators).

from openlayers.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.