Skip to content
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

Make the Map pickable #1813

Closed
BastienGauthier opened this issue Oct 6, 2023 · 8 comments
Closed

Make the Map pickable #1813

BastienGauthier opened this issue Oct 6, 2023 · 8 comments
Labels
enhancement Feature request or idea about how to make folium better

Comments

@BastienGauthier
Copy link
Contributor

BastienGauthier commented Oct 6, 2023

Is your feature request related to a problem? Please describe.
As an extension of python-visualization/branca#99, I want to be able to cache a map for an application, to switch quickly between them.

Describe the solution you'd like
I proposed a first solution in the following PR : #1812
The solution is partial and would greatly appreciate any help, as I feel a pickable Map could be helpful in many ways.

Additional context
Map before correction :
image

Map after correction
image

Implementation
See #1812

@Conengmo Conengmo added enhancement Feature request or idea about how to make folium better work in progress Work is in progress on a PR, check the PR to see its status labels Oct 11, 2023
@BastienGauthier
Copy link
Contributor Author

Ok, seems to work now ! The solution proposed needs branca modification python-visualization/branca#144 though.
I tested with some elements but I did not go though everything, even though a made the modification everywhere. If I export the html from the map, the files before pickling and after unpickling matches.

@Conengmo
Copy link
Member

Interesting! I haven't looked into the changes very thoroughly, but I wanted to challenge first already:

Is it possible to do this with changes just in Branca's Element class? Not in every class? So no adding template_str and no adding __set_state__ everywhere.

  • Since everything inherits from Element, changing getstate and setstate there should automatically work for every Folium class as well
  • Can you get the template string from an initialized Template object? That way there's no need to add template_str everywhere. You could extract the string from the Template object in getstate.

I haven't tried this myself so these are questions for you! Curious to hear what is possible.

@BastienGauthier
Copy link
Contributor Author

Indeed, I just tried and I can still simplify the setstate I will update the PR soon !
Regarding the template string, I could not find any way to extract it. I will be happy to implement it if you find something on your side !

@Conengmo
Copy link
Member

Too bad we can't extract the template string from the Template objects. I'd still like to avoid having to update every class though by specifying template_str everywhere.

Maybe as an alternative we can remove the _template object when pickling, then when unpickling re-add it from the class? When we call setstate, we know what class we have, and _template is available as a class attribute.

@BastienGauthier
Copy link
Contributor Author

Not sure how to add it from the class actually : do you have an exemple ?

@Conengmo
Copy link
Member

I looked into it a bit more, learned more about pickling and unpickling. Forget what I said in my previous comment. I learned that class attributes are not pickled, so we don't have to worry about _template as a class attribute. Only the template argument of Element is problematic, but I see you got that covered in python-visualization/branca#144!

I also realised we can save ourselves some trouble by not adding ENV as an instance attribute in the first place. I don't see a good reason to that. We have it as a module attribute, why add it to class instances? It's a private attribute even, so we don't expect users to use it. Let's not do that in your open PR, but I propose we do a separate PR and don't add ENV to any classes.

@BastienGauthier is python-visualization/branca#144 ready for review?

@BastienGauthier
Copy link
Contributor Author

Yes, ready to go !

@Conengmo
Copy link
Member

This was implemented in python-visualization/branca#144.

@Conengmo Conengmo removed the work in progress Work is in progress on a PR, check the PR to see its status label Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request or idea about how to make folium better
Projects
None yet
Development

No branches or pull requests

2 participants