Div icon

[ocefpaf]

@BibMartin this is my first attempt to dive in the new code. So I have a few questions:

• The original DivIcon class was inheriting from MacroElement. I changed it to Icon, although I don't think it matters in this case.
• You carefully tried to preserve the method div_markers, but I am starting to think that should go away. It is not really what the original leaflet divIcon is all about.
• If we agree that div_markers should ho we can safely remove the static_div_icon.js and the tests for that method.
• I believe there more <templates>.js that we can remove.

Note that in this PR I already removed the template. To see the failure keeping that go to:

https://travis-ci.org/ocefpaf/folium/jobs/91228684

🇫🇷 I am praying for the victims of this horrible attack. I Hope everybody close to you are alright. 🇫🇷

[ocefpaf]

This PR is the first one of a long term plan to get us closer to matplotlib via mplexporter and mplleaflet.

See: http://nbviewer.ipython.org/gist/ocefpaf/ab173ae7a7ee40430cdf

[BibMartin]

🇫🇷 I am praying for the victims of this horrible attack. I Hope everybody close to you are alright. 🇫🇷

Thank you for your prayers ; everyone I know is fine, but I have a deep thought for past and future victims.

[BibMartin]

This PR is the first one of a long term plan to get us closer to matplotlib via mplexporter and mplleaflet.

That's good idea ; I guess we can end-up with a MatplotlibFigure object that does mplleaflet's job.
I wonder whether we shall do this in calling mplleaflet, or get inspiration from it and do it in folium's style.

[themiurgo]

Great PR @ocefpaf . I love the notebook with arrow DivIcons, you just gave me a great idea to visualise GPS trajectories. 👍

@BibMartin There's nothing else to add to what has been already said in these days. Let's all hope in a better future. 🇫🇷

[ocefpaf]

you just gave me a great idea to visualise GPS trajectories.

Glad that was helpful! My real intent is to show ocean currents though 😉 (Soon in a notebook example.)

[ocefpaf]

That's good idea ; I guess we can end-up with a MatplotlibFigure object that does mplleaflet's job.
I wonder whether we shall do this in calling mplleaflet, or get inspiration from it and do it in folium's style.

I like the idea of a MatplotlibFigure. I think that the best course of action is to factor out the fig2json.
The fig2json is pretty simple and the only dependency is mplexporter and of course matplolitb.

PS: I removed the div_markers method and tests. I added some tests for the new DivIcon.
Looking at this again I really don't think we should inherent from icon. Most of we inherent is not applicable to the DivIcon (rotation, color, etc). What do you think? Should I just inherit from MacroElement?

[ocefpaf]

• The original DivIcon class was inheriting from MacroElement. I changed it to Icon, although I don't think it matters in this case.

@BibMartin I changed it back to MacroElement because, even though divIcon is an Icon, it does not make sense to expose the Icon methods here.

Let me know how bad this is and if we can salvage some ideas for a better PR 😬

[BibMartin]

What does # noqa mean ?

[ocefpaf]

No questions asked. It serves to tell humans and automatic tools to not mess with that line.

[BibMartin]

@ocefpaf
It looks fine to me. I just don't understand why you've dropped out the class structure in test_features.py and unstacked everything in different functions. But I guess this is not the active part of the PR.

Let me know how bad this is and if we can salvage some ideas for a better PR 😬

It's good to me. Just maybe I should write proper code so that you don't need to pep8 and docstring it.

[ocefpaf]

I just don't understand why you've dropped out the class structure in test_features.py and unstacked everything in different functions.

Something about pytest was not working properly. I can't remember what. However, I believe that pytest is designed to work better that way.

I will wait for #261 and #262. (Then I will go back to this one, rebase, polish, and ping you again 😄)

[ocefpaf]

Rebased and good for another round if review.

[BibMartin]

Merged : thanks @ocefpaf