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

Unable to assign string directly to popup using master branch #259

Closed
jelmelk opened this issue Nov 27, 2015 · 5 comments
Closed

Unable to assign string directly to popup using master branch #259

jelmelk opened this issue Nov 27, 2015 · 5 comments
Labels
enhancement Feature request or idea about how to make folium better

Comments

@jelmelk
Copy link
Contributor

jelmelk commented Nov 27, 2015

Doing some small testing and I did the follwing:

some_marker = folium.Marker(location=[45.5215, -122.6261], popup='test')

I would get this error: AttributeError: 'str' object has no attribute 'get_name'. If I change that to popup=folium.Popup('test') it works fine. Is that intended?

Main reason I'm asking is because it seems somewhat tedious having to call that to assign a string to the popup.

@jelmelk jelmelk changed the title Unable to call popup using master branch Unable to assign string to popup using master branch Nov 27, 2015
@jelmelk jelmelk changed the title Unable to assign string to popup using master branch Unable to assign string directly to popup using master branch Nov 27, 2015
@BibMartin
Copy link
Contributor

@jelmelk

Is that intended?
Main reason I'm asking is because it seems somewhat tedious having to call that to assign a string to the popup.

In the background, we cannot do without the Popup object, because one may be willing to create more complicated popups (with a vega chart for instance).

But I agree it's tedious, and in the foreground we can shortcut things in the Marker code to catch cases when the arg popup is a str (or bytes), and to replace it by Popup(popup).

It'd be great if you did a PR for that (and thus join the happy community of contributors). Otherwise, I'll do it on Monday.

@jelmelk
Copy link
Contributor Author

jelmelk commented Nov 27, 2015

I'm looking at that right now. Would it be better to change each marker individually since the error rises at map.py on line 389 for Marker and element.py on line 414 for CircleMarker etc...

They all have element.py line 46 in common which is where the exception is actually being called from. What would be the preference in this case?

@BibMartin
Copy link
Contributor

I imagine you can simply fix it in the Marker.__init__ is testing when isinstance(popup,text_type) or isinstance(popup,str_type) and then overwrite the popup attribute.

@themiurgo
Copy link
Contributor

We should strive to generalise this, while we're at it. You might want to have a popup also for Polylines, for example.

@ocefpaf ocefpaf added this to the v0.2.0 milestone Dec 3, 2015
@ocefpaf ocefpaf added the enhancement Feature request or idea about how to make folium better label Dec 3, 2015
@BibMartin
Copy link
Contributor

Fixed by #283. Closing this.

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

4 participants