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

Specify a default for property functions when property value is undefined #4124

Closed
lucaswoj opened this issue Feb 1, 2017 · 11 comments
Closed
Assignees
Labels
cross-platform 📺 Requires coordination with Mapbox GL Native (style specification, rendering tests, etc.)

Comments

@lucaswoj
Copy link
Contributor

lucaswoj commented Feb 1, 2017

From @lbud on August 16, 2016 20:30

In the mapbox-gl-function feature-defaults branch I'm adding reference as an argument so that when an input to a property function is undefined, it defaults to the spec default. However, it seems to me there should also be a way to specify a user-defined default for this behavior. Does it seem reasonable to add an optional key in a property function for default, like

extrusion-height: {
    stops: [[0, 0], [1000, 3000]],
    type: "exponential",
    property: "levels",
    default: 20
}

(so any extrusion without a levels property will have a height of 20, rather than the spec-defined default of 10)?

cc @lucaswoj @mollymerp @mourner

Copied from original issue: mapbox/mapbox-gl-style-spec#480

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @mourner on August 16, 2016 20:35

I like the idea. default property name can be confusing though, since it can be confused with extrusion-height default, not the default for the input property of a function. Alternatives? propertyDefault?

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @lbud on August 16, 2016 20:58

propertyDefault (or property-default — hyphens would be more consistent in this spec, I think) sounds good to me.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @jfirebaugh on August 22, 2016 18:19

Could we unify the syntax with the proposals for token defaults in #104?

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @lbud on August 26, 2016 20:26

@jfirebaugh do you mean something like

extrusion-height: {
    stops: [[0, 0], [1000, 3000]],
    type: "exponential",
    property: "levels:20"
}

? (It feels weird to use a number in a string there…)

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @jfirebaugh on August 26, 2016 20:37

No, I mean something akin to the syntaxes proposed in mapbox/mapbox-gl-style-spec#362 (comment) or mapbox/mapbox-gl-style-spec#104 (comment), like "property": {"ref": "levels", "default": 20} or "property": [{"ref": "levels"}, 20].

I think at this point we need to have a vision that unifies mapbox/mapbox-gl-style-spec#480, mapbox/mapbox-gl-style-spec#104, and mapbox/mapbox-gl-style-spec#47 or the function syntax will wind up becoming a mess of ad hoc additions.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @lbud on September 16, 2016 19:17

Interestingly, I realized that because of the way evaluateExponentialFunction picks a function stop — iterating over them, waiting for either a break statement or the end — we can exploit it to set, essentially, a default:

{
    "stops": [[0, 0], [1000, 1000], ["other", 20]]
    "property": "height"
}

I don't feel like this is really the intended behavior or a clean solution, buuuut… 💭

@jfirebaugh @lucaswoj

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @lbud on September 19, 2016 23:33

Actually, my above comment is not true — it was working in my debugging but now I'm creating demos and realizing validation throws out non-number stops in number functions, and non-ascending-order stops (so -1 doesn't work). I can still achieve this effect by ensuring that the second to last stop is a number beyond what would ever exist in the data, and then making the next number even larger: stops: [[0, 0], [1000, 1000], [2000, 20]] but this feels even more unclean.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

I think there are two distinct cases being discussed in this thread:

1. Specify a function input value when the property is undefined

This is being discussed in mapbox/mapbox-gl-style-spec#104. I will weigh in on that ticket.

2. Specify a function output value when the property is undefined

"extrusion-height": {
    stops: [[0, 0], [1000, 3000]],
    type: "exponential",
    property: "levels",
    default: 20
}

any extrusion without a levels property will have a height of 20, rather than the spec-defined default of 10

In some cases, we can take care of case 2 with a carefully constructed value and the case 1 feature. In other cases, doing so is impossible or mathematically tricky. For example:

"fill-color": {
    stops: [["scrub", "yellow"], ["industrial", "purple"], ...],
    default: "white",
    type: "categorical",
    property: "class"
}

We should have a primitive for this case. @lbud's syntax proposal is 👍

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @jfirebaugh on September 20, 2016 21:36

@lucaswoj Good clarification -- let's focus this issue on adding the ability to provide an output value for inputs which are not otherwise in the domain of the function.

An input value of "undefined", when the property is not present on a given feature, is a common special case. But a categorical function which doesn't wish to specify a stop for every possible value is another use case.

Then to go back to @mourner's first comment:

default property name can be confusing though, since it can be confused with extrusion-height default, not the default for the input property of a function

If we are talking about extrusion-height output default, not a default input, I think default is fine -- it's the same concept as a default label in a case statement, for example.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @davidtheclark on December 3, 2016 19:28

A clarification question: By "undefined" are we referring to both undefined and null? Or is null going to have to be a separate conversation?

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @jfirebaugh on December 5, 2016 18:14

A clarification question: By "undefined" are we referring to both undefined and null? Or is null going to have to be a separate conversation?

We're not referring to either a literal undefined or null input value, but rather to the case where the feature properties have no key-value pair for the desired property at all.

null is a separate conversation. Functions and filters don't currently have a notion of null values, largely because neither does the vector tile specification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cross-platform 📺 Requires coordination with Mapbox GL Native (style specification, rendering tests, etc.)
Projects
None yet
Development

No branches or pull requests

3 participants