Repository: geopandas/geopandas

Issue:
Improving reading from remotes and file-like objects
I'm happy to see progress in the ability to load from file-like objects in #1329. I've been interested in improving the ability for geopandas to load from file-like objects, especially those produced by [`fsspec` ](https://filesystem-spec.readthedocs.io), which should increase the universe of storage backends that we can use here. Since `fsspec` and OGR/Fiona have mutually incompatible virtual filesystems and URL compounding schemes, I think that proceeding with file-like objects is probably the best way to have these two libraries talk to each other.

When I looked into this, I found that the code paths for file loading in GeoPandas are a bit muddled, and could probably use some clarification. Here is a brief survey of the different scenarios I've found. The relevant code block is 
https://github.com/geopandas/geopandas/blob/d3228ddb6814515506921ad5c2e6d6f8bd99c9ae/geopandas/io/file.py#L85-L97
I'll be referring to the three branches of this if-elif-else block below.

### Loading a local text file
```python
geopandas.read_file("test.geojson")
```
This is likely the most common usage. It takes the third branch, and fiona opens the file by path.

### Loading a local binary file
```python
geopandas.read_file("gdal_sample.gpkg", driver="GPKG")
```
This also takes the third branch, and fiona opens the file by path.

### Loading a local zip file:
```python
geopandas.read_file("zip://shapefile.zip")
```
This takes the third branch. Note, however, that I need to include the fiona `zip://` scheme to tell fiona to open it as a zipped file. This is unsurprising, except see below.

### Loading a remote text file
```python
geopandas.read_file("https://opendata.arcgis.com/datasets/76104f230e384f38871eb3c4782f903d_13.geojson")
```
This takes the first branch, loading the GeoJSON into a `fiona.BytesCollection`.

### Loading a remote zipped file
```python
geopandas.read_file("https://opendata.arcgis.com/datasets/76104f230e384f38871eb3c4782f903d_13.zip")
```
This takes the first branch. Note the surprising behavior that *this zipped file doesn't need the `zip://` scheme*. As far as I can tell, this is because fiona's `BytesCollection` does some [automagic detection](https://github.com/Toblerity/Fiona/blob/4750ae99119c7bd2bc38d7842fd57c1c2db2cd2e/fiona/collection.py#L533-L536) of a zipped collection of bytes. The difference between URI schemes for remote and local zipped  files is pretty confusing, and not really documented anywhere, I think. Indeed, if I include `zip://` in the URL above, it fails.

### Loading a `StringIO` file-like object
```python
with open("test.geojson", "r") as f:
    gdf = geopandas.read_file(f, driver="GeoJSON")
```
This takes the second branch, as intended by #1329.

### Loading a `BytesIO` file-like object
```python
with open("gdal_sample.gpkg", "rb") as f:
    gdf = geopandas.read_file(f, driver="GPKG")
```
This *does not* take the second branch, as the `instanceof` check only looks for text objects. Instead it takes the third branch, which I think is not intended. However, fiona has some understanding of file-like objects as well, and it works anyways.

### Loading a zipped `BytesIO` file-like object
This may or may not be an important case, but I was unable to get this to work. In the future, I would hope to be able to get this working using `fsspec`.


Okay, so that is a whole lot of different behaviors, including a couple that I think are probably not intended. Additionally, it seems that @sgillies intends to deprecate `BytesCollection` in favor of `MemoryFile` at some point in the future, though I'm not sure when.  I don't know what the best way forward is right now, but I think there are a couple of behaviors that would be good to get right:
1. I think that having a more duck-typed check for file-like objects (e.g., `pandas.api.types.is_file_like()`) is probably worthwhile. Then it might work better for things like `BytesIO` or `fsspec`. Alternatively, maybe fiona's file-like logic is sufficient?
1. Ideally the differences in how zipped shapefiles are handled could be smoothed out, though I'm not sure how right now.


Discussion:
martinfleis: Looking good so far. Feel free to ignore CI with python 3.5 (#1492). There is also an unrelated issue with arrow (#1536). 
ian-r-rose: Sorry for the slow update here @martinfleis, this should be close-to-ready. It tries to cover the two main issues discussed in #1531 

* More general file-like objects can be read than just those deriving from `TextIO`. I have tested using `fsspec`, but others should work as well.
* We try to infer if a `zip://` scheme should be added to local files in order to match the behavior of remote files, which don't need that scheme.

I see that I still have a windows path-related failure to address, but comments would be appreciated.
ian-r-rose: Okay, here is a summary of the behavior as of this PR:

* If the filename is URL-like, geopandas will download the file and feed the bytes to fiona to be opened. Zipped files are automatically recognized.
* If the filename is a file-like object, its bytes are read and fed to fiona to be opened. Zipped files are automatically recognized.
* If the file is a path to a local file on disk, that path is passed to fiona for opening. If the path to the file ends with `".zip"`, then a `zip://` scheme is added to the path. This is to match the behavior of the above two code paths, so in all instances users should not need to add `zip://`.
* If the path is a GDAL-style `/vsi` virtual filesystem path, then it is passed to fiona unmodified.

ian-r-rose: Thanks for the review @jorisvandenbossche. I know this one got a bit more complex than I intended (who knew that path handling had edge cases?), so happy for suggestions of how to simplify.
jorisvandenbossche: > I know this one got a bit more complex than I intended (who knew that path handling had edge cases?), so happy for suggestions of how to simplify.

No problem! Filesystems / path handling is not the simplest topic ;)

The best way to simplify this is if fiona would do this automatic handling of zip files ;-) Something to discuss with them, but in the meantime, I think it would be good to already handle it in geopandas. It will make the experience with reading zip files a lot smoother! (it's a long time item on my list with small things I want to improve in geopandas ;-))
jorisvandenbossche: @ian-r-rose One more thing, could you add an example of using ffspec at the end of this section:https://geopandas.readthedocs.io/en/latest/docs/user_guide/io.html#reading-spatial-data ? There is already one using `open`, so it can fit after that one (the page is a bit messy, we need to reorganize that page, but at least having a small example ensures we include it when redoing the page)
ian-r-rose: Example added to the docs (with some fun filesystem chaining)

Patch:
diff --git a/ci/travis/36-minimal.yaml b/ci/travis/36-minimal.yaml
index c7b37397..3533fc12 100644
--- a/ci/travis/36-minimal.yaml
+++ b/ci/travis/36-minimal.yaml
@@ -14,6 +14,7 @@ dependencies:
   - pytest
   - pytest-cov
   - codecov
+  - fsspec
   # optional
   - rtree
   - matplotlib
diff --git a/ci/travis/36-pd025.yaml b/ci/travis/36-pd025.yaml
index 713078a5..cf5df536 100644
--- a/ci/travis/36-pd025.yaml
+++ b/ci/travis/36-pd025.yaml
@@ -12,6 +12,7 @@ dependencies:
   # testing
   - pytest
   - pytest-cov
+  - fsspec
   #- codecov
   # optional
   - rtree
diff --git a/ci/travis/37-dev.yaml b/ci/travis/37-dev.yaml
index 7dd3f16c..2b463640 100644
--- a/ci/travis/37-dev.yaml
+++ b/ci/travis/37-dev.yaml
@@ -13,6 +13,7 @@ dependencies:
   # testing
   - pytest
   - pytest-cov
+  - fsspec
   #- codecov
   # optional
   - rtree
diff --git a/doc/source/docs/user_guide/io.rst b/doc/source/docs/user_guide/io.rst
index b22b0dd6..b5826b3f 100644
--- a/doc/source/docs/user_guide/io.rst
+++ b/doc/source/docs/user_guide/io.rst
@@ -57,6 +57,14 @@ as a file handler (e.g. via built-in ``open`` function) or ``StringIO``::
     file = open(filename)
     df = geopandas.read_file(file)
 
+File-like objects from `fsspec <https://filesystem-spec.readthedocs.io/en/latest>`_
+can also be used to read data, allowing for any combination of storage backends and caching
+supported by that project::
+
+    path = "simplecache::http://download.geofabrik.de/antarctica-latest-free.shp.zip"
+    with fsspec.open(path) as file:
+        df = geopandas.read_file(file)
+
 You can also read path objects::
 
     import pathlib
diff --git a/geopandas/geodataframe.py b/geopandas/geodataframe.py
index 0038aee4..a764b178 100644
--- a/geopandas/geodataframe.py
+++ b/geopandas/geodataframe.py
@@ -192,7 +192,7 @@ class GeoDataFrame(GeoPandasBase, DataFrame):
         Parameters
         ----------
         col : column label or array
-        drop : boolean, default True
+        drop : boolean, default False
             Delete column to be used as the new geometry
         inplace : boolean, default False
             Modify the GeoDataFrame in place (do not create a new object)
diff --git a/geopandas/io/file.py b/geopandas/io/file.py
index 2a94f3f3..080935a2 100644
--- a/geopandas/io/file.py
+++ b/geopandas/io/file.py
@@ -1,6 +1,5 @@
 from distutils.version import LooseVersion
 
-import io
 import warnings
 import numpy as np
 import pandas as pd
@@ -36,6 +35,16 @@ def _is_url(url):
         return False
 
 
+def _is_zip(path):
+    """Check if a given path is a zipfile"""
+    parsed = fiona.path.ParsedPath.from_uri(path)
+    return (
+        parsed.archive.endswith(".zip")
+        if parsed.archive
+        else parsed.path.endswith(".zip")
+    )
+
+
 def _read_file(filename, bbox=None, mask=None, rows=None, **kwargs):
     """
     Returns a GeoDataFrame from a file or URL.
@@ -85,10 +94,30 @@ def _read_file(filename, bbox=None, mask=None, rows=None, **kwargs):
         req = _urlopen(filename)
         path_or_bytes = req.read()
         reader = fiona.BytesCollection
-    elif isinstance(filename, io.TextIOBase):
-        path_or_bytes = filename.read()
-        reader = fiona.open
+    elif pd.api.types.is_file_like(filename):
+        data = filename.read()
+        path_or_bytes = data.encode("utf-8") if isinstance(data, str) else data
+        reader = fiona.BytesCollection
     else:
+        # Opening a file via URL or file-like-object above automatically detects a
+        # zipped file. In order to match that behavior, attempt to add a zip scheme
+        # if missing.
+        if _is_zip(str(filename)):
+            parsed = fiona.parse_path(str(filename))
+            if isinstance(parsed, fiona.path.ParsedPath):
+                # If fiona is able to parse the path, we can safely look at the scheme
+                # and update it to have a zip scheme if necessary.
+                schemes = (parsed.scheme or "").split("+")
+                if "zip" not in schemes:
+                    parsed.scheme = "+".join(["zip"] + schemes)
+                filename = parsed.name
+            elif isinstance(parsed, fiona.path.UnparsedPath) and not str(
+                filename
+            ).startswith("/vsi"):
+                # If fiona is unable to parse the path, it might have a Windows drive
+                # scheme. Try adding zip:// to the front. If the path starts with "/vsi"
+                # it is a legacy GDAL path type, so let it pass unmodified.
+                filename = "zip://" + parsed.name
         path_or_bytes = filename
         reader = fiona.open
 
diff --git a/geopandas/tools/clip.py b/geopandas/tools/clip.py
index da76e0f2..75ef3ce6 100644
--- a/geopandas/tools/clip.py
+++ b/geopandas/tools/clip.py
@@ -127,7 +127,7 @@ def clip(gdf, mask, keep_geom_type=False):
     if not isinstance(mask, (GeoDataFrame, GeoSeries, Polygon, MultiPolygon)):
         raise TypeError(
             "'mask' should be GeoDataFrame, GeoSeries or"
-            "(Multi)Polygon, got {}".format(type(gdf))
+            "(Multi)Polygon, got {}".format(type(mask))
         )
 
     if isinstance(mask, (GeoDataFrame, GeoSeries)):


Test patch:
diff --git a/ci/travis/37-latest-conda-forge.yaml b/ci/travis/37-latest-conda-forge.yaml
index 80ff961b..71f87fca 100644
--- a/ci/travis/37-latest-conda-forge.yaml
+++ b/ci/travis/37-latest-conda-forge.yaml
@@ -13,6 +13,7 @@ dependencies:
   - pytest
   - pytest-cov
   - codecov
+  - fsspec
   # optional
   - rtree
   - matplotlib
diff --git a/ci/travis/37-latest-defaults.yaml b/ci/travis/37-latest-defaults.yaml
index 5a8839a2..ad96f2c6 100644
--- a/ci/travis/37-latest-defaults.yaml
+++ b/ci/travis/37-latest-defaults.yaml
@@ -12,6 +12,7 @@ dependencies:
   # testing
   - pytest
   - pytest-cov
+  - fsspec
   #- codecov
   # optional
   - rtree
diff --git a/ci/travis/38-latest-conda-forge.yaml b/ci/travis/38-latest-conda-forge.yaml
index d19642f2..951d7a41 100644
--- a/ci/travis/38-latest-conda-forge.yaml
+++ b/ci/travis/38-latest-conda-forge.yaml
@@ -13,6 +13,7 @@ dependencies:
   - pytest
   - pytest-cov
   - codecov
+  - fsspec
   # optional
   - rtree
   - matplotlib
diff --git a/geopandas/io/tests/test_file.py b/geopandas/io/tests/test_file.py
index 236c856d..09b5a355 100644
--- a/geopandas/io/tests/test_file.py
+++ b/geopandas/io/tests/test_file.py
@@ -304,6 +304,16 @@ def test_read_file_remote_geojson_url():
     assert isinstance(gdf, geopandas.GeoDataFrame)
 
 
+@pytest.mark.web
+def test_read_file_remote_zipfile_url():
+    url = (
+        "https://raw.githubusercontent.com/geopandas/geopandas/"
+        "master/geopandas/datasets/nybb_16a.zip"
+    )
+    gdf = read_file(url)
+    assert isinstance(gdf, geopandas.GeoDataFrame)
+
+
 def test_read_file_textio(file_path):
     file_text_stream = open(file_path)
     file_stringio = io.StringIO(open(file_path).read())
@@ -356,6 +366,47 @@ def test_read_file_tempfile():
     temp.close()
 
 
+def test_read_binary_file_fsspec():
+    fsspec = pytest.importorskip("fsspec")
+    # Remove the zip scheme so fsspec doesn't open as a zipped file,
+    # instead we want to read as bytes and let fiona decode it.
+    path = geopandas.datasets.get_path("nybb")[6:]
+    with fsspec.open(path, "rb") as f:
+        gdf = read_file(f)
+        assert isinstance(gdf, geopandas.GeoDataFrame)
+
+
+def test_read_text_file_fsspec(file_path):
+    fsspec = pytest.importorskip("fsspec")
+    with fsspec.open(file_path, "r") as f:
+        gdf = read_file(f)
+        assert isinstance(gdf, geopandas.GeoDataFrame)
+
+
+def test_infer_zipped_file():
+    # Remove the zip scheme so that the test for a zipped file can
+    # check it and add it back.
+    path = geopandas.datasets.get_path("nybb")[6:]
+    gdf = read_file(path)
+    assert isinstance(gdf, geopandas.GeoDataFrame)
+
+    # Check that it can sucessfully add a zip scheme to a path that already has a scheme
+    gdf = read_file("file+file://" + path)
+    assert isinstance(gdf, geopandas.GeoDataFrame)
+
+    # Check that it can add a zip scheme for a path that includes a subpath
+    # within the archive.
+    gdf = read_file(path + "!nybb.shp")
+    assert isinstance(gdf, geopandas.GeoDataFrame)
+
+
+def test_allow_legacy_gdal_path():
+    # Construct a GDAL-style zip path.
+    path = "/vsizip/" + geopandas.datasets.get_path("nybb")[6:]
+    gdf = read_file(path)
+    assert isinstance(gdf, geopandas.GeoDataFrame)
+
+
 def test_read_file_filtered(df_nybb):
     full_df_shape = df_nybb.shape
     nybb_filename = geopandas.datasets.get_path("nybb")