Plug elements in 0.1.* API

[BibMartin]

(the 36 first commits are the same as #203)

As written in #203, this breaks the 0.1.* code in order to plug elements.py.

A few tests are passing and a lot of them are failing.

The goal is to adapt/pass the 30 existing test and ensure :

• that you can still use the former API, with deprecation warnings
• that the new API si used everywhere in the background.

[ocefpaf]

@BibMartin you are accumulating a lot of work here and it is getting hard to review as time passes. Yous plans are to merge #203 first, right?

[BibMartin]

@ocefpaf Yes, I plan to have #203 merged and then rebase this one to hide the commits they have in common.
btw : this PR is indeed half-finished (I opened it to give visibility on where I'm going) ; I expect (hope) it will be easier to review when finished.

[ocefpaf]

I expect (hope) it will be easier to review when finished.

Thanks! The problem to review is on my side. (Lack of time 😒)

[BibMartin]

Can you please have a look at former commit (BibMartin@6832e24). I wonder whether these two tests (test_geo_json_bad_color and test_geo_json_bad_threshold_scale) are still useful ❓

[ocefpaf]

Both test_geo_json_bad_color and test_geo_json_bad_threshold_scale are probably * no longer useful. They might be in a choropleth method that may or may not be derived from the geo_json method. I still need to think a little bit more on how to do this properly.

* I got a GitHub Sorry, this diff is temporarily unavailable due to heavy server load. ❗

[BibMartin]

@ocefpaf

They might be in a choropleth method that may or may not be derived from the geo_json method. I still need to think a little bit more on how to do this properly.

For the moment you can do a choropleth in doing

m = Map(...)
g = GeoJson(...)                   # Create the geo_json shapes
g.add_children(GeoJsonStyle(...)   # Define the colouring
m.add_children(g)
m.add_children(ColorScale(...))    # Set the legend

Of course, we will need a shortcut method. Maybe simply

m = Map(...)
m.add_children(GeoJson(..., style=GeoJsonStyle(...), legend=ColorScale(...)))

Or as before, m.geo_json(...) is not crazy to me.

[ocefpaf]

Or as before, m.geo_json(...) is not crazy to me.

The thing is. We either let the user be responsible for ColorScale or we create a convenience choropleth method to adjust the ColorScale automatically*. I would like geo_json to just add the GeoJSON assuming very little about the underlying data.

* We could even use some of PySAL methods to determine the ColorScales.

[BibMartin]

✅ Green at last ! And ready for review...
@ocefpaf @themiurgo if you have time...

[ocefpaf]

Awesome! I will take a look before the weekend.

Thanks a lot for the hard work @BibMartin!

[ocefpaf]

I guess you do not need that slash. right?

[BibMartin]

I guess you're right. It's a (bad?) habbit I took not to rely on brackets for multi-line.
Tell me and I'll put them away.

[ocefpaf]

I'd rather not have them there. But that is just cosmetics... We still need a better review than mine to get this merged as soon as possible!

[ocefpaf]

I am 👍

However, I am a bit overwhelmed with all the changes and I'd really appreciate more eyes on this PR before we get it merged.

@themiurgo Do you think you can take a quick look at this? If not, @BibMartin since it has been so long you wrote this I guess you are now an independent observer of your own code 😛 so take a second look and give me the green to merge.

[BibMartin]

Thanks @ocefpaf for first review.

If not, @BibMartin since it has been so long you wrote this I guess you are now an independent observer of your own code 😛 so take a second look and give me the green to merge.

I will try the exercise, but I would be more confident if @themiurgo had time to comment : it's not only technical changes but also a different mindset ; I can challenge my code, but barely my own philosophy

[ocefpaf]

Last call for reviews 😉

Merging this Tomorrow!

[ocefpaf]

Thanks @BibMartin 🎉