-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
[WIP] add cluster mapreduce feature to style spec #7004
Conversation
I fixed the flow tests, but I'm having trouble with the unit tests. I now get stuff like |
This is really awesome — thank you very much for taking on this! I like that the implementation is relatively short — not as scary as I thought it would be. I'll review this in more detail this week. |
Amazing @redbmk! This prompted me to start thinking about how the design of the reduce expressions should look, and I filed #7010. I think that along with the the "initial": [ "{}", "max", 1, "sum", 0 ],
"map": [ "{}",
"max", ["get", "scalerank"],
"sum", ["get", "scalerank"]
],
"reduce": [ "fn", "cluster", "point",
"{}",
"max", ["max",
["get", "max", ["var", "cluster"]],
["get", "max", ["var", "point"]]
]
"sum": ["+",
["get", "sum", ["var", "cluster"]],
["get", "sum", ["var", "point"]]
]
] |
@anandthakker I like it - that would be pretty cool and actually simplify the cluster code and the style spec. Would there be a way in the style spec to specify that Technically |
Any update on that @mourner and @redbmk ? I am currently trying to have data-driven paint attributes for my clusters. I've seen that using supercluster was an option since I want to use other features that just point_counts, but the only example out there using supercluster is broken and out-of-date. Would very much like to see how one could achieve that either by using the MR work shown here or with a separate use of supercluster. See the related Stack Overflow question: |
Awesome, we need such kind of feature to show minimum and maximum prices of cluster |
Continued in #7584. |
Launch Checklist
@mapbox/studio
and/or@mapbox/maps-design
if this PR includes style spec changesNotes
This should fix #2412. It works from the testing I've done so far, and I changed the cluster test to use this feature.
This is still a WIP because I just threw this together without really discussing the implementation (e.g. does it make sense to use
point
andcluster
variables, or is there another way to do this?). Also, I still need to do some benchmarking and finish documenting the new features (I've added some documentation to the style spec, but I'd imagine there's more to do than that). Any help would be much appreciated!I'm not sure how to tag @mapbox/studio and @mapbox/maps-design - those seem relevant since this affects the style spec, but github doesn't autocomplete when I type those, and they don't appear as links when I check
Preview
. Does anybody know how to tag them like the PR template suggests?@mourner It might also be good to add access to
zoom
at some point, which would involvesupercluster
changes (essentially just also passing inzoom
to the_accumulate
function).@anandthakker I wanted to add some global variables for
Infinity
and-Infinity
in case you want to do something likemin
,max
without setting the initial value to0
(e.g. what if you want the max, but all the numbers are negative?). I couldn't figure out how to do this though. I tried setting the globals to include them like in the following snippet, but then when I tried to access them with["get", "infinity"]
or["get", "-infinity"]
, I gotnull
. Do you know the right way to support those?Maybe we could add them as properties, and then have the actual properties of the point be exposed in
point
(currently that's how it is in thereduce
function, but not inmap
). That would give us variablespoint
(with the point's properties),infinity
, and-infinity
(and onreduce
we would also havecluster
).New Feature Description
This adds a new field to a GeoJSON source called
clusterMapReduce
, which will allow you to use mapbox expressions to pass data intosupercluster
'sinitial
,map
, andreduce
options. The usage would look like this:In order to use
supercluster
's mapreduce, the important property isreduce
. By default,initial
will be an empty object ({}
), andmap
will return the point's properties. So, for example if you haveprice
as a property on a point, and you want the clusters to have a propertyprice
that is actually the min of those, you could simply do:In
reduce
, the accumulated cluster is exposed as the variablecluster
, and the current point is exposed aspoint
.In
map
, the properties are exposed as as regular properties, so you don't need to nest theget
(i.e. you can do["get, "value"]
instead of["get", "value", ["get", "point"]]
).In
initial
, you don't have access to any feature, but expressions are still supported.