streams are over roads

[eringreb]

Streams show overtop of trails and roads.

[nvkelso]

Oops, definitely a bug!

On Thu, Mar 9, 2017 at 5:36 PM, eringreb notifications@github.com wrote:

Streams show overtop of trails and roads.

[image: image] https://cloud.githubusercontent.com/assets/18540280/23778392/df172aee-0507-11e7-9489-43c86a91ab13.png

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/tangrams/walkabout-style/issues/151, or mute the thread https://github.com/notifications/unsubscribe-auth/AA0EOzF8bZeF8mNDugEZRjUskhlXaK4Oks5rkKkqgaJpZM4MY26u .

[nvkelso]

And @eringreb congrats for your first Github issue! :)

[eringreb]

Thanks! The map looks great. Also made me realize I tagged a bunch of trails in my local parks incorrectly since they weren't showing up colored. Fixed now!

[nvkelso]

:)

On Fri, Mar 10, 2017 at 11:12 AM, eringreb notifications@github.com wrote:

Thanks! The map looks great. Also made me realize I tagged a bunch of trails in my local parks incorrectly since they weren't showing up colored. Fixed now!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/tangrams/walkabout-style/issues/151#issuecomment-285758106, or mute the thread https://github.com/notifications/unsubscribe-auth/AA0EO6hYttj9qTWQEw5K7vaxHvfmBn_Uks5rkaCxgaJpZM4MY26u .

[sensescape]

@bcamper (Nathaniel writing) can you check the logic here, please? It seems like Tangram isn't evaluate the true and false correctly?

There is a vector tile bug (https://github.com/tilezen/vector-datasource/issues/1143) that streams that are intermittent=false shouldn't have that property exported at all, but if Tangram sees false in the data, shouldn't that evaluate false in the test? Right now any and all waterways are drawing dashed instead :\

• https://github.com/tangrams/walkabout-style/blob/gh-pages/walkabout-style.yaml#L1502

intermittent:
                filter:
                    any:
                        - intermittent: true
                        - kind: drain
                draw:
                    lines:
                        visible: false
                    dots-lines:
                        order: function() { return feature.sort_rank; }
                        color: [[10,[0.472,0.834,0.890]],[14,[0.511,0.877,0.930]]]
                        width: [[10,0px],[11,0.5px],[12,1px],[13,2px],[14,3px],[15,4px],[16,5px]]

[bcamper]

Boolean filters actually behave differently, as covered in the docs ;)

https://mapzen.com/documentation/tangram/Filters-Overview/#feature-properties

A Boolean value of “true” will pass a feature that contains the named property, ignoring the property’s value. A value of “false” will pass a feature that does not contain the named property:

filter:
    kind: true
    area: false

To match a property whose value is a boolean, use the list syntax:

filter:
    boolean_property: [true]

This is a quirk going back to early Tangram. I honestly think that I meant to implement it to match JS truthy/falsiness (e.g. "true" values include true but also 1, random string, etc., "false" values include false but also null, 0, etc.)... but it was implemented as existence-of-property instead and carried to ES that way.

I don't really like it, but we've avoided changing it to not break compatibility, and because there was an existing syntax that did support direct boolean matches (the list syntax with a single boolean value works because lists matches the property value to any of the values in the list).

So you can make this:

intermittent:
                filter:
                    any:
                        - intermittent: [true]
                        - kind: drain

I would also be interested in revisiting this syntax though since I am skeptical that it has every been used (or understood!) as currently written. cc @matteblair @meetar

[bcamper]

@matteblair If I recall, you had some qualms about trying to reimplement all of JS truthy/falsey behavior in C++, an understandable hesitation given that itself a quirk of the language :) A more modest change could be to make it so that a boolean filter matches take false values into account:

• true filter matches any value that is neither null (current behavior) nor false (new behavior)
• false filter matches any value that is null (current behavior) or false (new behavior)

[nvkelso]

@bcamper Thanks for the background, that makes a lot more sense now.

Fixed the intermittent part in 33d17d41046b3887296d12859db65b4156381101.

This fixes the line ordering for the generic stream (solid) line, but the dashed case would still overlap roads because of the blend mode, which may be unavoidable for now.

[bcamper]

You can't set the blend_mode on the dash line so they're under the roads (maybe not without breaking something else)?

[matteblair]

I feel like there may be more usage of this syntax than we realize - I'd want to take a cursory survey of its use before we seriously consider changing it.

@bcamper In your suggested revision I think you wrote null where you meant undefined, which is important because the two "values" are different in the YAML core schema.

[matteblair]

Which means that tag: true would still match a value of null, since there is a defined value for tag that is not literally false.

I don't think the current syntax is all that bad, but if we were to change it I would favor something more explicit in syntax. We could use a mapping to check existence, similar to the range filter syntax: tag: { defined: true }.

[bcamper]

I did mean null, but I should have also included undefined.

Which means that tag: true would still match a value of null, since there is a defined value for tag that is not literally false.

I don't think so? I had:

true filter matches any value that is neither null (current behavior) nor false (new behavior)

"neither null nor false", meaning the only values that tag: true would not match are null and false (maybe I just phrased awkwardly). My original suggestion should be amended to:

• tag: true matches any value that is not undefined, null, or false
• tag: false matches any value that is undefined, null, or false

Agree on syntax usage review "in the wild" before changing, just making proposals here. I would say I like your suggestion of something like tag: { defined: true }, but my main point here is that I believe that this syntax is likely both generally misunderstood (and thus used incorrectly/unintentionally), and that there is not a common practical use case for this feature (e.g. I don't think data often uses the presence of absence of a field to indicate something, it's more common this would be done through a boolean flag as in the example in this ticket... omitting the field entirely is an optimization to save space in the data, but the proposed syntax revisions here would match both a present false value, or a missing value).

So rather than supporting special syntax for this, I'd prefer to allow people who truly need that to do so with a function filter like filter: function(){ return feature.tag != null } -- support the common case with built-in boolean filter syntax, and the uncommon case with JS function.

[matteblair]

I see, I misunderstood the revision you were suggesting.

• tag: true matches any value that is not undefined, null, or false
• tag: false matches any value that is undefined, null, or false

This seems harder to understand than the current syntax; instead of this filter meaning one thing about a feature, it would now mean one of three possible things about the feature.

If using Boolean feature properties is more common than using the presence/absence of properties, then wouldn't it make sense to use the more concise true/false syntax to denote actual Boolean values?