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

Support style dictionary #817

Merged
merged 3 commits into from
Apr 14, 2022
Merged

Support style dictionary #817

merged 3 commits into from
Apr 14, 2022

Conversation

banesullivan
Copy link
Contributor

Resolves #730

I changed a few tests at random to use a dict instead of doing json.dumps to make sure this is tested

@manthey
Copy link
Member

manthey commented Apr 13, 2022

My one concern is with tile source caching. Currently, the style parameter is used for caching -- if the dictionary is identical enough, then it works as expected, but if the style dictionary differs, the caching may be less than expected (e.g., have equivalent dictionaries whose keys are differently ordered). I'm wondering if it would be worth LRU hashing function so that if style is a dictionary it is serialized to json with sorted keys rather than just used as a python repr.

Or, maybe we don't care about efficiency in that case because a client is unlikely to reorder a style dictionary between tile source accesses.

@banesullivan
Copy link
Contributor Author

Or, maybe we don't care about efficiency in that case because a client is unlikely to reorder a style dictionary between tile source accesses.

I think it's an unlikely edge case that the dictionary keys will be differently ordered between requests

(e.g., have equivalent dictionaries whose keys are differently ordered). I'm wondering if it would be worth LRU hashing function so that if style is a dictionary it is serialized to json with sorted keys rather than just used as a python repr.

Should we just convert the dict to an OrderedDict?: https://docs.python.org/3/library/collections.html#collections.OrderedDict

and perhaps also sort the JSON blob as well?

@manthey
Copy link
Member

manthey commented Apr 14, 2022

When we generate the JSON blob from the dict, we are ordering it. As of Python 3.6, all dictionaries are ordered . I think I'll merge this -- if anyone ever mutates the order of the keys in the dict and notices caching misses, we'll handle it then.

@manthey manthey merged commit 251d57d into master Apr 14, 2022
@manthey manthey deleted the feat/dict-style branch April 14, 2022 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow dict for style
2 participants