Skip to content

Fix some typos and pep8 #182

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

Merged
merged 1 commit into from
Aug 16, 2015
Merged

Conversation

ocefpaf
Copy link
Member

@ocefpaf ocefpaf commented Aug 14, 2015

I was fixing #181 and started to pep8 and fix typos... So I decided to send this as a PR first and then the actual fix. @BibMartin or @themiurgo can either of you review and merge this one?

@ocefpaf ocefpaf force-pushed the pep8 branch 2 times, most recently from 06ad850 to a44d452 Compare August 16, 2015 12:29
@ocefpaf
Copy link
Member Author

ocefpaf commented Aug 16, 2015

Re-based and good to go.

@@ -404,9 +413,12 @@ def div_markers(self, locations=None, popups=None, marker_size=10, popup_width=3
'icon': "{'icon':"+icon_name+"}"
})

popup_out = self._popup_render(popup=popup, mk_name='div_marker_{0}_'.format(call_cnt),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you fix this line like this instead of something like

mk_out = 'div_marker_{0}_'.format(call_cnt)
popup_out = self._popup_render(popup=popup, mk_name=mk_out)

?

I don't know pep8 very well, but it seems to me that it would be more understandable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. Much better. I am just going to call it mk_name instead of mk_out to be more consist.

@BibMartin
Copy link
Contributor

2 comments in the code ; as for the rest, it seems good to me.

@BibMartin
Copy link
Contributor

Ok, let's merge 🎉

BibMartin added a commit that referenced this pull request Aug 16, 2015
Fix some typos and pep8
@BibMartin BibMartin merged commit b76b08f into python-visualization:master Aug 16, 2015
@ocefpaf ocefpaf deleted the pep8 branch August 16, 2015 17:17
@ocefpaf
Copy link
Member Author

ocefpaf commented Aug 16, 2015

Thanks. That kind of cleanup was in my TODO list for folium since day 1 (well.. It is python so day 0)!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants