-
Notifications
You must be signed in to change notification settings - Fork 4
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
refactor SEPAL map #463
refactor SEPAL map #463
Conversation
It will be supporting the drawing methods (editing, polygonize) from there
Codecov Report
@@ Coverage Diff @@
## master #463 +/- ##
==========================================
+ Coverage 89.07% 90.33% +1.26%
==========================================
Files 26 31 +5
Lines 3093 3290 +197
==========================================
+ Hits 2755 2972 +217
+ Misses 338 318 -20
Continue to review full report at Codecov.
|
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.
I've made some comments in SepalMap. I'll wait until the "max_zoom"
error is fixed so I can test the map on live, and I will keep testing the new feats.
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.
I just made a single comment but I'm approving the review. It looks really nice, great job prr!
Fix #422
Fix #456
Main
The main objectives of this PR are to remove all dependencies to geemap as the number of functionalities provided is growing out of control and we are not using 10% of them. Also I took the occasion to partially revamp everything that was in mapping
To test the new code, you can use the dataset I provide in the following GIST: https://gist.github.com/12rambau/852d82c3b2c53ceee4a05c0f4cf01d9a
detail
find_layer
method it accept every possible key: layer, layer_name, index. return None if nothing is foundfind_layer
I overwrittenremove_layer
to use the same parameter (index, layer, layer_name). raise an error if nothing is foundremove_all
long wanting (at least from me) method to remove all layer from the map (leaving the basemaps).DrawControl
from the SepalMap. it will be way easier to solve customize the drawControl object to manage edition (not natively set up) #386 from therelocal_layer
andee_layer
as they are uselessEELayer
to store the original dataset used in the layers (mainly for the v_inspector)LocaleTileLayer
(same reasons)v_inspector
fromSepalMap
and revamped the whole thing (I think it's nicer, and it's able to read the 4 types of data)MapBtn
that use the appropriate styling params to be responsive and nicely display on the map. as it's a vuetify element we can use icons from latest fas/mdi. I changed both the v_inspector and the fullscreen to use it. I'll handle the issue of the tooltip in another PR.TODO
conclusion
I don't want to add new things (tooltips, edit in drawcontrol, fullscreen bug) to this PR as it would make it way too big. I'm happy to receive feedbacks on the new component to correct them
Some of the libs start to be deprecating python 3.6 I think we'll drop the support starting from this PR