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

Add cenpy & osmnx example notebook #148

Merged
merged 4 commits into from
Oct 9, 2021
Merged

Conversation

ljwolf
Copy link
Member

@ljwolf ljwolf commented May 13, 2020

This adds an example with using contextily to add tiles to an osmnx and cenpy-generated map of Austin, TX.

It also has black ran on the docs/conf.py file.

@ljwolf ljwolf requested a review from darribas May 13, 2020 15:36
Copy link
Collaborator

@darribas darribas left a comment

Choose a reason for hiding this comment

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

Super, my only comment would be to change the main title of the notebook to something like "cenpy & osmnx and then adding the current title as a subtitle (on GEE I've used four #). That way the title pulled in for the menu bar is shorter and easier to render. Otherwise good to go!

@ljwolf
Copy link
Member Author

ljwolf commented May 14, 2020

Thanks! I've shortened the main title an added a clearer/more descriptive subtitle as an h4.

@jorisvandenbossche
Copy link
Member

One small comment might also be that the section explaining what contextily is, might be unneeded for a notebook in the contextily docs ;)

@ljwolf
Copy link
Member Author

ljwolf commented May 14, 2020

sure sure, I only put that in to ensure that if it is linked to without context, it's got at least some explanation. Happy to drop that!

@darribas
Copy link
Collaborator

I think this is cool as is. One suggestion to extend it (maybe add below rather than edit the map that's there) could be:

  • Use a dark basemap (Carto's dark matter?)
  • "Transfer" the values from the Census geographies to the street network
  • Make a choropleth of the street network on top of contextily

If you like this, wanna have a go at this and think will have time soon, I'd say go ahead and we merge everything at once. But it you don't see yourself with bandwith with this any time soon, I'd say let's merge this and "if it happens" in the future, we can always extend the notebook, that's not a problem.

@darribas
Copy link
Collaborator

darribas commented Oct 9, 2021

Merging this and we can pick up that to-do list above in a different PR

@darribas
Copy link
Collaborator

darribas commented Oct 9, 2021

Merging as failed tests aren't related to this PR

@darribas darribas merged commit 5b560f9 into geopandas:master Oct 9, 2021
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.

3 participants