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

Added GeoPandas data interface #88

Merged
merged 12 commits into from
Nov 3, 2017
Merged

Added GeoPandas data interface #88

merged 12 commits into from
Nov 3, 2017

Conversation

philippjfr
Copy link
Member

Adds a GeoPandas interface allowing Path and Polygon types to wrap around geopandas dataframes directly. Here is a basic example:

https://anaconda.org/philippjfr/geopandas/notebook

Requires holoviews master and will likely be released at the same time as HoloViews 1.9.

@jbednar
Copy link
Member

jbednar commented Sep 14, 2017

Excellent! Would it be difficult to make it work with Matt Rocklin's new cythonized geopandas equivalent as well, which appears to have vastly better performance than bare geopandas?

@philippjfr
Copy link
Member Author

Would it be difficult to make it work with Matt Rocklin's new cythonized geopandas equivalent as well, which appears to have vastly better performance than bare geopandas?

I assume that will eventually be folded into geopandas proper? As long as the API is similar/the same it should be trivial either way. That said if you really need the optimization then your data is likely larger than bokeh can comfortably render.

@mrocklin
Copy link

No particular thoughts on this yet. We've been talking about getting coordinates out of GeoPandas a bit to speed up operations like this. I would not be surprised if we have a different API for this in the future, but I wouldn't wait on geopandas development.

I would be curious to know if you run into performance issues on larger datasets.

@philippjfr
Copy link
Member Author

philippjfr commented Nov 3, 2017

Probably won't have time to add tests here, I'll do that at the same time as moving the iris interface from HoloViews. Otherwise this is ready to review @jbednar. The PR also removes a lot of plotting code relying instead on an approach that simply projects the element and then uses the HoloViews version of the plot to actually plot the element. This means that in most cases plotting classes consist solely of a declaration of the projection operation. There are now projection operations for most elements and I'll finish off better Image and RGB handling in #87.

@philippjfr
Copy link
Member Author

Can close #53 once merged.

@jbednar
Copy link
Member

jbednar commented Nov 3, 2017

Looks good, apart from the failing test. It would be nice to add a sentence or two saying why to use GeoPandas, i.e. what you gain and tradeoffs there are if any.

@philippjfr
Copy link
Member Author

Very confused by the latest error, dask seems to be raising an error about np.cbrt not being defined:

  File "/home/travis/miniconda/envs/test-environment/lib/python2.7/site-packages/dask/array/ufunc.py", line 138, in <module>
    cbrt = ufunc(np.cbrt)
AttributeError: 'module' object has no attribute 'cbrt'

@philippjfr
Copy link
Member Author

Looks like something in our travis setup is downgrading numpy to numpy 1.9.3-py27h7e35acb_3

@jbednar
Copy link
Member

jbednar commented Nov 3, 2017

That's odd; surely all Numpy versions have cbrt. Something's fishy.

@philippjfr
Copy link
Member Author

No, np.cbrt is a new feature in NumPy 1.13 so it makes sense.

@jbednar
Copy link
Member

jbednar commented Nov 3, 2017

Looks like cbrt was added in Numpy 1.10.0.

@jbednar
Copy link
Member

jbednar commented Nov 3, 2017

So dask should be specifying numpy>=1.10, I guess?

@philippjfr
Copy link
Member Author

Ah, sorry, I just searched the 1.13 changelog and didn't realize it listed all the changelogs.

@philippjfr
Copy link
Member Author

So dask should be specifying numpy>=1.10, I guess?

I suppose so, I think we may have a clash with some other dependency though, we'll see in a minute.

@philippjfr
Copy link
Member Author

In the .travis.yml you mean? I prefer pinning specific versions there, but it probably doesn't make much of a difference.

@jbednar
Copy link
Member

jbednar commented Nov 3, 2017

It's a tradeoff, I guess, between mistakenly thinking that a change we made caused a problem, when really it was a new release of numpy that did it, versus not realizing that users will have problems from new numpy releases when they do happen.

@philippjfr
Copy link
Member Author

Will go ahead and merge this so I can move on to #87.

@philippjfr philippjfr merged commit 27db4b7 into master Nov 3, 2017
@philippjfr philippjfr deleted the geopandas_interface branch January 13, 2018 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants