-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Use Jinja2 tojson
filter
#1101
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
Use Jinja2 tojson
filter
#1101
Conversation
""") | ||
{% macro script(this, kwargs) %} | ||
var {{ this.get_name() }} = new L.RegularPolygonMarker( | ||
{{ this.location|tojson }}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Way cleaner! I like this!!
I’ll have a look and fix the notebooks. After rebasing Python 2 should no longer be a problem :) This PR touches quite a lot of code. Do you see any problems with this? |
At a first glance, no. Even though it does touch a lot of the code base it is virtually the same changes everywhere, so let's trust our tests and merge this as soon as you rebase it and the tests are passing. |
Good point! During the work for this PR I looked at the tests more, and so I got thinking about want to look into having better functional tests. |
@ocefpaf seems the Travis build is failing on the installation step. It started from the last merged PR. It's hard to debug because it sees the whole block as one command:
|
Don't `json.dumps` options and data, but use Jinja2's `tojson` filter to convert Python variables to JS compatible formats. This works for strings, numbers, bools, lists and dictionaries. I added a test file to verify this behavior. Normalize the behavior of options. In `__init__`, put options in a `self.options` dictionary using a new `parse_options()` function. Also allow `**kwargs` whenever it makes sense. I also did refactoring of templates whenever I touched them. Have consistent indentation and style. Fixed the tests for the changes. Consistently use the `normalize` function to normalize rendered output.
If Numpy int32's are returned the `tojson` filter doesn't work.
investigating it. Hold on... |
@Conengmo the CIs are back online and the failure there is real ;-p |
Thanks for bringing the CI back! I see the test is indeed failing. It's due to the Webmapservice that's used in the WMS example notebook. This has happened before, now it seems to give a HTTP 503 instead of a timeout. I'll see if we can use another URL... |
Or we can just add an |
Don't
json.dumps
options and data, but use Jinja2'stojson
filter to convert Python variables to JS compatible formats. This works for strings, numbers, bools, lists and dictionaries. I added a test file to verify this behavior.Normalize the behavior of options. In
__init__
, put options in aself.options
dictionary using a newparse_options()
function. Also allow**kwargs
whenever it makes sense.This combination: consistent option handling and using
tojson
makes our code and templates more clean and consistent. Allowing**kwargs
gives users more flexibility.I also did refactoring of templates whenever I touched them. Have consistent indentation and style.
Fixed the tests for the changes. Consistently use the
normalize
function to normalize rendered output.