search

intake / intake_geopandas

An intake plugin for loading datasets with geopandas
BSD 2-Clause "Simplified" License
15 stars 7 forks source link

Multiple input sources #2

Closed ian-r-rose closed 5 years ago

ian-r-rose commented 5 years ago

Fixes #1.

Still need to add a version for PostGIS and write a bunch of tests, but I wanted to get your feedback sooner rather than later @jacobtomlinson. What do you think?

martindurant commented 5 years ago

I wouldn't bother explicitly labelling the superclass as abstract, since you might find later you want to have a geopandas-specific container (as opposed to general dataframe) and this would be the obvious place to do it.

ian-r-rose commented 5 years ago

I'm not sure I understand @martindurant, can you elaborate? My thought here was that these DataSources were per-data-source, and the GeoPandasSource was deferring how to load from different sources to its subclasses. This seems somewhat orthogonal to whether to elevate a GeoDataFrame to its own container-type.

martindurant commented 5 years ago

They are indeed two separate issues, and you are right in your understanding - I was just thinking that the superclass could also be the container class. Or maybe not - just a suggestion. I just personally never actually label a class explicitly as abstract, but maybe that's just my style.

ian-r-rose commented 5 years ago

Got it, thanks. I'm spending some significant time in Python right now after a long-ish time in Javascript, so I'm still playing around with style choices : )

ian-r-rose commented 5 years ago

ping @jacobtomlinson any thoughts here?

ian-r-rose commented 5 years ago

Thanks for taking a look @jacobtomlinson! I still have it marked as WIP as I had hoped to get some tests working for Spatialite at least. I've had some trouble getting that working in CI, though.

ian-r-rose commented 5 years ago

Okay, this is ready for another look. I've added a couple of SpatiaLite tests that are based off of ones in GeoPandas. Unfortunately, they rely on bugfixes that are not in a published version, but I have verified that they pass locally using geopandas master, and marked them as xfail for now.

jacobtomlinson commented 5 years ago

Released in 0.2.0

ian-r-rose commented 5 years ago

Thanks for the review and quick publish!

martindurant commented 5 years ago

Does this mean that intake-dcat can now be released?

ian-r-rose commented 5 years ago

I am waiting on dask/dask#4634, which is required for it to be able to access datasets from Socrata sites. Then I plan to publish a quick release to PyPI. It would be useful if we could also publish intake_geopandas there as well.

martindurant commented 5 years ago

Yes, please, to all of that!