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

DOC: update contextily example #1088

Merged
merged 2 commits into from Aug 12, 2019
Merged

Conversation

martinfleis
Copy link
Member

Updated contextily example to use add_basemap from contextily 1.0. However, I would wait with merging until contextily 1.0.0 is stable as pip install contextily now installs 0.99.0 which does not work with this example.

Closes #1077

@jorisvandenbossche
Copy link
Member

Yep, you're right about the pip issue. Another option could be to mention this explicitly in the beginning (and saying that pip users can upgrade with pip install --pre contextily)

@codecov
Copy link

codecov bot commented Aug 12, 2019

Codecov Report

Merging #1088 into master will increase coverage by 0.21%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1088      +/-   ##
==========================================
+ Coverage    90.2%   90.41%   +0.21%     
==========================================
  Files          19       20       +1     
  Lines        1837     1878      +41     
==========================================
+ Hits         1657     1698      +41     
  Misses        180      180
Impacted Files Coverage Δ
geopandas/__init__.py 100% <0%> (ø) ⬆️
geopandas/tools/_show_versions.py 100% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8c42164...ab1b198. Read the comment docs.

@martinfleis
Copy link
Member Author

pip install --pre contextily will make sense only for some time (I assume you want to release stable soon?). I can put a note that the example requires contextily >= 1.0.0?


ax = df.plot(figsize=(10, 10), alpha=0.5, edgecolor='k')
add_basemap(ax, zoom=10)
ctx.add_basemap(ax, zoom=12)
Copy link
Member

Choose a reason for hiding this comment

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

The original example of zoom=10 gave a less detailed background, with zoom=12 it actually takes some time ..

Do you think the higher zoom is better to illustrate? (if so, the longer doc build time is fine for me)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. In reality, you'll rather want more detailed map than blurry one. I tried to simulate real-life use. I am aware that it takes longer, but I feel it makes more sense this way.

@martinfleis martinfleis merged commit 7e6701a into geopandas:master Aug 12, 2019
@martinfleis martinfleis deleted the ctx_ex branch March 1, 2021 10:28
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.

DOC: update the contextily example in the gallery
2 participants