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.

Sign up for GitHub

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

Jump to bottom

Added default options to tile_layer #236

Merged
merged 2 commits into from
Nov 9, 2015
Merged

Added default options to tile_layer #236

merged 2 commits into from
Nov 9, 2015

Conversation

ocefpaf
Copy link
Member

@ocefpaf ocefpaf commented Nov 4, 2015

Hopefully the last step to close v.0.1.6.

@ocefpaf ocefpaf mentioned this pull request Nov 4, 2015
Closed
@ocefpaf
Copy link
Member Author

ocefpaf commented Nov 4, 2015

I added the commit to fix the typos and pep8 from #234

@ocefpaf ocefpaf added this to the v0.1.6 milestone Nov 6, 2015
@ocefpaf
Copy link
Member Author

ocefpaf commented Nov 8, 2015

Ping @andrewgiessel or @BibMartin to review and merge. Once this one gets merged I will make a new PyPI release and be done with v0.1.6!

BibMartin
BibMartin reviewed Nov 9, 2015
View reviewed changes
folium/folium.py
zoomOffset = kw.pop('zoomOffset', 0)
zoomReverse = kw.pop('zoomReverse', 'false')
continuousWorld = kw.pop('continuousWorld', 'false')

Copy link
Contributor

Choose a reason for hiding this comment

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

Provided that you handle when tms,noWrap,zoomReverse,continuousWorld are bool, I would set defaults to False instead of 'false'. What do you think ?
Anyway, this is a matter of taste and obviously not blocking for merge.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree. I was not comfortable with that either.

Done and rebased!

@BibMartin
Copy link
Contributor

I'm okay for merge ; thank @ocefpaf for this piece of work (not the funniest, btw).
One small (non-blocking) comment in the code.
Merge in 12 hours 🕐

BibMartin added a commit that referenced this pull request Nov 9, 2015
@BibMartin
Merge pull request #236 from ocefpaf/default_opt
8664f09 
Added default options to tile_layer
@BibMartin BibMartin merged commit 8664f09 into python-visualization:v0.1.6 Nov 9, 2015
@BibMartin
Copy link
Contributor

merged. Thanks again @ocefpaf

@ocefpaf ocefpaf deleted the default_opt branch November 9, 2015 20:06
@ocefpaf
Copy link
Member Author

ocefpaf commented Nov 9, 2015

Baking a new release now!

@ocefpaf ocefpaf added the enhancement Feature request or idea about how to make folium better label Feb 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement Feature request or idea about how to make folium better
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ocefpaf @BibMartin @ozak