Skip to content
This repository has been archived by the owner on Jan 12, 2024. It is now read-only.

Support modulo in selectors #377

Closed
systemed opened this issue Nov 7, 2014 · 8 comments
Closed

Support modulo in selectors #377

systemed opened this issue Nov 7, 2014 · 8 comments
Milestone

Comments

@systemed
Copy link

systemed commented Nov 7, 2014

Mapnik supports the numeric modulo (%) operator in selectors: https://github.com/mapnik/mapnik/wiki/Filter#examples-in-xml

For example:

<Filter>[height] % 50 = 0</Filter> 

Carto support for this would be good.

@springmeyer
Copy link

@systemed would 9459084 work?

@systemed
Copy link
Author

systemed commented Jul 9, 2015

Looks great.

@nebulon42 nebulon42 added this to the 0.18 milestone Jan 20, 2017
@pnorman
Copy link
Contributor

pnorman commented May 11, 2017

Was a PR ever done for the branch @springmeyer did?

@nebulon42
Copy link
Collaborator

No, because the syntax is a bit incomplete. When you are only able to specify x % 2 without anything else it is unclear what you are filtering for.

@nebulon42 nebulon42 modified the milestones: 0.18, 1.0 May 11, 2017
@gravitystorm
Copy link
Contributor

I've run into the need for this too. I think @nebulon42 is correct about the suggested patch - it makes something like this work:

<Filter>[height] % 50</Filter> 

whereas what we want is slightly different:

<Filter>[height] % 50 = 0</Filter> 

The latter is the example used in the mapnik documenation for filters.

As far as I can figure out, the problem seems to be in carto's parsing of the filter. It expects to see something like key operator value. In our example modulo is part of the key, not an operator. So I think we want to end up with:

  • [height] % 50 is the key
  • = is the operator
  • 0 is the value

I can't figure out if one of the existing five tests for keys should be modified to allow the above, or a sixth test added. Any guidance on the right approach would be welcome - I'm a bit out of my depth here.

@gravitystorm
Copy link
Contributor

I tried adding this.expression to the list of key tests, and while that made it pass this example, it broke 40+ other tests :-(

@nebulon42
Copy link
Collaborator

If we allow expressions in filters we might have to start writing fields explicitly with [field] syntax. When changing

if (key = $(/^[a-zA-Z0-9\-_]+/) ||
to $(this.expression) the failing tests complain about keywords. So the parsers does not recognize height in height = 0 as field anymore. Writing [height] = 0 fixes that.

But I only had a quick look. Maybe there is a better alternative.

@nebulon42
Copy link
Collaborator

One has the be careful in which order the expression is added for the parser. Seems to work now, only restriction is that field references have to be properly written within brackets e.g. [[height] % 50 = 0].

gravitystorm pushed a commit to gravitystorm/carto that referenced this issue Jan 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants