Skip to content
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

Rework spec function/expression taxonomy (recreate #6505) #6521

Merged
merged 12 commits into from
May 7, 2018
Merged

Conversation

lbud
Copy link
Contributor

@lbud lbud commented Apr 16, 2018

Recreates #6505 — I reverted until I have a gl-native PR ready, as it'll otherwise cause a lot of code generation errors for the next person who tries to update the gl-js pin there.

New since #6505 I've also added a list of property-types and descriptor docs in a75b518 [reverted]


This PR reworks the taxonomy to describe how a property can use functions/expressions, as described in #6389 (comment) and #6389 (comment):

type ExpressionType = 'data-driven' | 'cross-faded' | 'cross-faded-data-driven' | 'color-ramp' | 'data-constant' | 'constant';

type ExpressionParameters = Array<'zoom' | 'feature' | 'heatmap-density' | 'line-progress'>;

type ExpressionSpecification = {
    interpolated: boolean,
    parameters: ExpressionParameters
}

StylePropertySpecification = {
    ...
    'property-type': ExpressionType,
    'expression': ExpressionSpecification
}

so now any property all properties include a property-type key, and for all property-types except constant also an expression property, i.e.

    "type": "number",
    "property-type": "data-driven",
    "expression": {
        "interpolated": true,
        "parameters": [
            "zoom",
            "feature"
        ]
    }

Benchmarks: http://bl.ocks.org/lbud/raw/2ac3b8869074c112802166978cea8345/ (I couldn't reproduce the Layer discrepancies on subsequent isolated bench runs).

Closes #6389
Supersedes / closes #4194
Refs #6430
Refs #6303

Launch Checklist

  • briefly describe the changes in this PR
  • write tests for all new functionality
  • post benchmark scores
  • manually test the debug page

@lbud
Copy link
Contributor Author

lbud commented Apr 17, 2018

@samanpwbb @anandthakker @jfirebaugh @1ec5 requesting review from anyone based on the new additions to this since #6505 (noted at the top of OP) — format + language of the property-type docstrings added + style-spec docs page format.

@jfirebaugh
Copy link
Contributor

I don't think we should add these to the documentation. We already have fairly complex terminology surrounding properties ("layout", "paint", "function", "expression", "interpolatable", "data expression", "camera expression", ...), and I don't think additional terminology is going to clarify the situation. Let's keep these types as an implementation detail.

lbud pushed a commit to mapbox/mapbox-gl-native that referenced this pull request May 4, 2018
Ports mapbox/mapbox-gl-js#6521: for the sake of gl-native this only requires tweak to code generation scripts.
@lbud
Copy link
Contributor Author

lbud commented May 7, 2018

I reverted the addition of property-types on the docs page in 5bd3e06. The only changes thereafter from the previous iteration of this PR (#6505) are related to changes in master since then that would otherwise cause tests in gl-native to fail when updating the gl-js pin.

@lbud lbud merged commit 5b3af8e into master May 7, 2018
@lbud lbud deleted the expression-spec branch May 7, 2018 22:44
lbud pushed a commit to mapbox/mapbox-gl-native that referenced this pull request May 14, 2018
Ports mapbox/mapbox-gl-js#6521, updating codegen scripts to parse new expression taxonomy.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants