Use Jinja2 `tojson` filter #1101

Conengmo · 2019-03-16T22:40:03Z

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.

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.

folium/raster_layers.py

tests/plugins/test_polyline_text_path.py

ocefpaf · 2019-03-17T12:48:38Z

folium/features.py

-            """)
+        {% macro script(this, kwargs) %}
+            var {{ this.get_name() }} = new L.RegularPolygonMarker(
+                {{ this.location|tojson }},


Way cleaner! I like this!!

ocefpaf · 2019-03-17T13:14:04Z

@Conengmo this is failing legacy Python and the notebook tests. The former we no longer care, #1100 was already merged BTW, but the latter seems to be related to the changes here, right?

Conengmo · 2019-03-17T14:20:32Z

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?

ocefpaf · 2019-03-17T14:23:13Z

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.

Conengmo · 2019-03-17T14:28:22Z

let's trust our tests

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.

Conengmo · 2019-03-17T18:29:18Z

@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:

The command "wget http://bit.ly/miniconda -O miniconda.sh
  bash miniconda.sh -b -p $HOME/miniconda
  export PATH="$HOME/miniconda/bin:$PATH"
  conda config --set always_yes yes --set changeps1 no --set show_channel_urls true
  conda update conda --quiet
  conda config --add channels conda-forge --force
  conda install pycryptosat
  conda config --set channel_priority strict
  conda config --set safety_checks disabled
  conda create --name TEST python=$PY --file requirements.txt --file requirements-dev.txt
  source activate TEST
  " failed and exited with 1 during .

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.

ocefpaf · 2019-03-17T20:26:50Z

@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:

investigating it. Hold on...

ocefpaf · 2019-03-18T14:00:55Z

@Conengmo the CIs are back online and the failure there is real ;-p

Conengmo · 2019-03-18T19:44:48Z

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...

ocefpaf · 2019-03-18T19:46:02Z

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 xfail (expected failure) there. I'll see if I can do that later today. It is unrelated to this PRs anyway so let's get this merged!

stickler-ci reviewed Mar 16, 2019

View changes

folium/raster_layers.py Outdated Show resolved Hide resolved

tests/plugins/test_polyline_text_path.py Outdated Show resolved Hide resolved

Conengmo force-pushed the Conengmo:jinja2-tojson branch from 1739267 to e83f813 Mar 16, 2019

ocefpaf reviewed Mar 17, 2019

View changes

folium/features.py

""")

{% macro script(this, kwargs) %}

var {{ this.get_name() }} = new L.RegularPolygonMarker(

{{ this.location|tojson }},

This comment has been minimized.

Sign in to view

ocefpaf Mar 17, 2019
Member

Way cleaner! I like this!!

Test Jinja2 tojson behavior

a35199d

Conengmo force-pushed the Conengmo:jinja2-tojson branch from e83f813 to 0de5581 Mar 17, 2019

Conengmo force-pushed the Conengmo:jinja2-tojson branch from 0de5581 to 2bfd656 Mar 17, 2019

Coordinate validation should return floats

Loading status checks…

12b8c58

If Numpy int32's are returned the `tojson` filter doesn't work.

Conengmo force-pushed the Conengmo:jinja2-tojson branch from 2bfd656 to 12b8c58 Mar 17, 2019

Conengmo deleted the Conengmo:jinja2-tojson branch Mar 18, 2019

FabeG mentioned this pull request Mar 19, 2019

Adds PolyLineOffset Plugin #1091

Merged

python-visualization / folium

Use Jinja2 `tojson` filter #1101

Use Jinja2 `tojson` filter #1101

Conengmo commented Mar 16, 2019

This comment has been minimized.

ocefpaf commented Mar 17, 2019

Conengmo commented Mar 17, 2019

ocefpaf commented Mar 17, 2019

Conengmo commented Mar 17, 2019

Conengmo commented Mar 17, 2019 •

edited

ocefpaf commented Mar 17, 2019

ocefpaf commented Mar 18, 2019

Conengmo commented Mar 18, 2019

ocefpaf commented Mar 18, 2019

python-visualization / folium

Join GitHub today

Use Jinja2 `tojson` filter #1101

Use Jinja2 `tojson` filter #1101

Conversation

Conengmo commented Mar 16, 2019

This comment has been minimized.

ocefpaf Mar 17, 2019 Member

ocefpaf commented Mar 17, 2019

Conengmo commented Mar 17, 2019

ocefpaf commented Mar 17, 2019

Conengmo commented Mar 17, 2019

Conengmo commented Mar 17, 2019 • edited

ocefpaf commented Mar 17, 2019

ocefpaf commented Mar 18, 2019

Conengmo commented Mar 18, 2019

ocefpaf commented Mar 18, 2019

Essential cookies

Always active

Analytics cookies

ocefpaf Mar 17, 2019
Member

Conengmo commented Mar 17, 2019 •

edited