Skip to content

#382: TopoJson.get_bounds() returns [lon,lat] instead of [lat,lon] #383

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
Merged
merged 1 commit into from
Mar 14, 2016
Merged

#382: TopoJson.get_bounds() returns [lon,lat] instead of [lat,lon] #383

merged 1 commit into from
Mar 14, 2016

Conversation

eddies
Copy link

@eddies eddies commented Mar 14, 2016

Fixes #382

BibMartin
BibMartin reviewed Mar 14, 2016
View reviewed changes
tests/test_folium.py
assert bounds == [[-124.56617536999985, 41.99187135900012],
[-116.46422312599977, 46.28768217800006]], bounds
assert bounds == [[41.99187135900012, -124.56617536999985,],
[46.28768217800006, -116.46422312599977]], bounds
Copy link
Contributor

Choose a reason for hiding this comment

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

To pass the tests, you just need to remove extra comma after -124.56617536999985.

@BibMartin
Copy link
Contributor

Thanks @eddies . Clean and awesome PR.
I'll merge it as soon as you've passed the tests (see comment in diff).

@ocefpaf This worth being cherry-picked on v0.2 branch, no ? Can you do it when it's merged on master, please ?

@eddies
Copy link
Author

eddies commented Mar 14, 2016

@BibMartin thanks for catching that--I had only ensured that py.test -s tests/ was passing for me locally so hadn't noticed.

Seems unlikely(?) that this latest CI failure (https://travis-ci.org/python-visualization/folium/jobs/115804229) is related to this PR. Can someone trigger a re-run?

@ocefpaf
Copy link
Member

ocefpaf commented Mar 14, 2016

Great work! Thanks for the PR! 👍

Seems unlikely(?) that this latest CI failure (https://travis-ci.org/python-visualization/folium/jobs/115804229) is related to this PR. Can someone trigger a re-run?

You are correct. The PR is fine and we need to fix that in another one.

@BibMartin feel free to merge. I will prep a v0.2.1 release with this fix.

BibMartin pushed a commit to BibMartin/folium that referenced this pull request Mar 14, 2016
Fix heatmap.fit_bounds (same bug as python-visualization#383)
b26a6e3
@BibMartin
Copy link
Contributor

I relaunched the tests and they pass now -> Merging. Thanks again @eddies .

BibMartin added a commit that referenced this pull request Mar 14, 2016
@BibMartin
Merge pull request #383 from eddies/382-topojson_bounds
be6d981 
#382: TopoJson.get_bounds() returns [lon,lat] instead of [lat,lon]
@BibMartin BibMartin merged commit be6d981 into python-visualization:master Mar 14, 2016
ocefpaf added a commit that referenced this pull request Mar 14, 2016
@ocefpaf
Merge pull request #384 from BibMartin/issue383bis
aff92a8 
Fix heatmap.fit_bounds (same bug as #383)
ocefpaf pushed a commit to ocefpaf/folium that referenced this pull request Mar 14, 2016
@ocefpaf
Fix heatmap.fit_bounds (same bug as python-visualization#383)
f446a7e
@eddies eddies deleted the 382-topojson_bounds branch March 14, 2016 15:40
ocefpaf pushed a commit to ocefpaf/folium that referenced this pull request Mar 14, 2016
@ocefpaf
Fix heatmap.fit_bounds (same bug as python-visualization#383)
b8e44f7
sanga pushed a commit to sanga/folium that referenced this pull request Oct 24, 2017
@BibMartin
Merge pull request python-visualization#383 from eddies/382-topojson_…
23abb0c 
…bounds

python-visualization#382: TopoJson.get_bounds() returns [lon,lat] instead of [lat,lon]
sanga pushed a commit to sanga/folium that referenced this pull request Oct 24, 2017
Fix heatmap.fit_bounds (same bug as python-visualization#383)
1328520
sanga pushed a commit to sanga/folium that referenced this pull request Oct 24, 2017
@ocefpaf
Merge pull request python-visualization#384 from BibMartin/issue383bis
32e5c06 
Fix heatmap.fit_bounds (same bug as python-visualization#383)
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
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@eddies @BibMartin @ocefpaf