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

clusterProperties does not fully support Style Specification. #1571

Closed
nflahavan opened this issue Sep 9, 2022 · 3 comments · Fixed by #1855
Closed

clusterProperties does not fully support Style Specification. #1571

nflahavan opened this issue Sep 9, 2022 · 3 comments · Fixed by #1855
Labels
bug 🪲 Something is broken!

Comments

@nflahavan
Copy link

Environment

  • Xcode version: 13.4.1
  • iOS version: 15.6.1
  • Devices affected: All devices are affected.
  • Maps SDK Version: v10.8.0

Observed behavior and steps to reproduce

it is not possible to express [["+", ["accumulated"], ["get", "sum"]], ["get", "scalerank"]] via Expression, therefore it is not possible to use [["+", ["accumulated"], ["get", "sum"]], ["get", "scalerank"]] to express a cluster property.

Expected behavior

I expect to be able to use [["+", ["accumulated"], ["get", "sum"]], ["get", "scalerank"]] as a value in the dictionary clusterProperties.

Notes / preliminary analysis

In order to express [["+", ["accumulated"], ["get", "sum"]], ["get", "scalerank"]] and other such expressions, the type for the value of the clusterProperties dictionary must support an array of two expressions.

I believe this can be accomplished by adding another Operator called expression with associated value of type Expression. This will require removing String raw type from the enum. Custom encoder/decoder function/init will need to be added as well.

Another possible solution is to change the type of clusterProperties to [String: [Expression.Element]]. This simple solution is less type safe but much easier to implement. It would be nice to make Expression.elements public if this route is taken.

Additional links and references

clusterProperties Style spec

clusterProperties API spec

@pjleonard37
Copy link
Contributor

pjleonard37 commented Jan 24, 2023

Hi @nflahavan --

Thanks for writing up this report and including all of this detail here. We've made some changes -- in this PR -- which bring clusterProperties into alignment with the Style Specification by enabling Expressions to be created without an operator. You should now be able to create Expressions like [["+", ["accumulated"], ["get", "sum"]], ["get", "scalerank"]] and use them with clusterProperties. As you can see, we took a different approach from the solutions you recommended above. If you have time to try out the changes we'd appreciate any feedback you have. They are available on the main branch now and will be included in an upcoming cycle of beta, rc, and stable releases.

Update: This change is now available in v10.11.0-rc.1.

@nflahavan
Copy link
Author

Hi @pjleonard37. I just got a chance to look at your pr and the solution makes sense to me! 👍

Thanks for putting in the work on this.

@pjleonard37
Copy link
Contributor

Glad to hear it - happy clustering!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Something is broken!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants