search

mapbox / supercluster

A very fast geospatial point clustering library for browsers and Node.
ISC License
2.1k stars 299 forks source link

point_count_abbreviated is specific to English #110

Closed 1ec5 closed 5 years ago

1ec5 commented 5 years ago

The point_count_abbreviated property is set to a string that abbreviates “thousands” as “k”. This convention is specific to English, whereas lots of languages have very different conventions for abbreviating thousands, sometimes depending on plurality.

https://github.com/mapbox/supercluster/blob/cd687abcff8de2109c7006ad5d75ee0e6329a72a/index.js#L321-L323

This library should either have built-in support for abbreviating based on CLDR rules or allow the developer to customize the format.

/cc @mourner @kkaefer @julianrex

mourner commented 5 years ago

The property was introduced before we implemented expessions in GL, so I'm only leaving it there for legacy compatibility reasons — I don't think we should introduce any customization options for it because you can do the same more flexibly with expressions (edit: if it's not flexible enough, I'd rather push for a proper format expression than build upon an outdated concept).

julianrex commented 5 years ago

Given that this is here for legacy reasons - I'm tempted to remove it from the iOS/macOS PR. Any reasons why it should stay?

1ec5 commented 5 years ago

The property was introduced before we implemented expessions in GL, so I'm only leaving it there for legacy compatibility reasons

Cool, that makes sense. It certainly is possible to implement the same functionality using expressions today: https://github.com/mapbox/mapbox-gl-native/pull/12952#discussion_r236455548. For mapbox/mapbox-gl-native#12952, I think we should omit the constant but continue to document the property as a legacy attribute in https://github.com/mapbox/mapbox-gl-native/pull/12952#discussion_r236393020.

if it's not flexible enough, I'd rather push for a proper format expression than build upon an outdated concept

I agree, mapbox/mapbox-gl-js#4119 would be the most correct solution for formatting these numbers, and it would benefit styling needs beyond clustering as well. Until then, we should document the expression from https://github.com/mapbox/mapbox-gl-native/pull/12952#discussion_r236455548 somewhere in the iOS/macOS map SDK’s headers or guides.