Repository: geopandas/geopandas

Issue:
MAINT: Fiona 1.10 support
In our CI builds our fiona tests are failing against the 1.10a wheels, as a new path opening API is introduced (https://github.com/Toblerity/Fiona/pull/1331/files). We use `fiona.parse_path`, `fiona.path.ParsedPath`, `fiona.path.UnparsedPath` and `fiona.path.ParsedPath.from_uri(path)` at a quick glance. Now I think if we'd imported `fiona.path` instead of getting it relative to `import fiona` we'd just hit a warning that these will be removed in fiona 2.0 instead of hard crashes, but we may as well look at solving the root problem right now.

Edit: had a bit of a look and it seems to keep the same level of convenience witth zipfiles we might need to either vendor some things from fiona (or use the private internals), or use fsspec or similar

Discussion:
jorisvandenbossche: > Not sure how we feel about introducing the new code / duplication.

I am personally fine with doing that. It's indeed some duplication, but then we also are more robust against future Fiona changes (and most of the effort of re-writing this outside of Fiona was already done in pyogrio)

To what extent did you cut down the pyogrio code? Would it be easier to just copy-paste it verbatim, even if that includes some features we don't need, so it's easier to update it later in case pyogrio would update / fix things?
jorisvandenbossche: > To what extent did you cut down the pyogrio code? Would it be easier to just copy-paste it verbatim, even if that includes some features we don't need, so it's easier to update it later in case pyogrio would update / fix things?

Ah, looking at the code in pyogrio, I suppose it's mostly the `buffer_to_virtual_file` call, that's indeed something we have to remove here (and some completely unrelated helpers like `_preprocess_options_key_value`, that's of course fine to leave out)
m-richards: > > To what extent did you cut down the pyogrio code? Would it be easier to just copy-paste it verbatim, even if that includes some features we don't need, so it's easier to update it later in case pyogrio would update / fix things?
> 
> Ah, looking at the code in pyogrio, I suppose it's mostly the `buffer_to_virtual_file` call, that's indeed something we have to remove here (and some completely unrelated helpers like `_preprocess_options_key_value`, that's of course fine to leave out)

The functions that we use are unchanged, it was just the imports and other unused stuff
jorisvandenbossche: Yeah, I essentially wrote that piece of code in pyogrio by taking fiona's code (at the time) and simplifying it for just what we needed (eg we didn't need to class structure for just the internal usage)
martinfleis: Can you re-enable that test path we limited to pyogrio in #3209? To test the original issue.

Patch:
diff --git a/ci/envs/310-pd20-conda-forge.yaml b/ci/envs/310-pd20-conda-forge.yaml
index 4854fdb8..91e55bc9 100644
--- a/ci/envs/310-pd20-conda-forge.yaml
+++ b/ci/envs/310-pd20-conda-forge.yaml
@@ -11,8 +11,7 @@ dependencies:
   - pyproj
   - packaging
   # testing
-  # TEMP pin, 8.1 breaks doctestplus, see https://github.com/scientific-python/pytest-doctestplus/issues/239
-  - pytest==8.0.*
+  - pytest
   - pytest-cov
   - pytest-xdist
   - fsspec
diff --git a/geopandas/io/file.py b/geopandas/io/file.py
index e03c623b..a3cd6ddc 100644
--- a/geopandas/io/file.py
+++ b/geopandas/io/file.py
@@ -6,6 +6,7 @@ import warnings
 
 import numpy as np
 import pandas as pd
+from geopandas.io.util import vsi_path
 from pandas.api.types import is_integer_dtype
 
 import shapely
@@ -57,6 +58,7 @@ def _import_fiona():
             FIONA_GE_19 = Version(Version(fiona.__version__).base_version) >= Version(
                 "1.9.0"
             )
+
         except ImportError as err:
             fiona = False
             fiona_import_error = str(err)
@@ -175,18 +177,6 @@ def _is_url(url):
         return False
 
 
-def _is_zip(path):
-    """Check if a given path is a zipfile"""
-    import fiona.path
-
-    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, engine=None, **kwargs):
     """
     Returns a GeoDataFrame from a file or URL.
@@ -322,27 +312,7 @@ def _read_file_fiona(
         # 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(path_or_bytes)):
-            # TODO: disconnect GeoPandas from Fiona's URI/path parsing internals.
-            # Meanwhile parse_path is imported here to avoid adding more conditional
-            # statements at the top of the module.
-            from fiona.path import parse_path
-
-            parsed = parse_path(str(path_or_bytes))
-            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)
-                path_or_bytes = parsed.name
-            elif isinstance(parsed, fiona.path.UnparsedPath) and not str(
-                path_or_bytes
-            ).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.
-                path_or_bytes = "zip://" + parsed.name
+        path_or_bytes = vsi_path(str(path_or_bytes))
 
     if from_bytes:
         reader = fiona.BytesCollection
diff --git a/geopandas/io/util.py b/geopandas/io/util.py
new file mode 100644
index 00000000..2bfbee8d
--- /dev/null
+++ b/geopandas/io/util.py
@@ -0,0 +1,118 @@
+"""Vendored, cut down version of pyogrio/util.py for use with fiona"""
+
+import re
+import sys
+from urllib.parse import urlparse
+
+
+def vsi_path(path: str) -> str:
+    """
+    Ensure path is a local path or a GDAL-compatible vsi path.
+
+    """
+
+    # path is already in GDAL format
+    if path.startswith("/vsi"):
+        return path
+
+    # Windows drive letters (e.g. "C:\") confuse `urlparse` as they look like
+    # URL schemes
+    if sys.platform == "win32" and re.match("^[a-zA-Z]\\:", path):
+        if not path.split("!")[0].endswith(".zip"):
+            return path
+
+        # prefix then allow to proceed with remaining parsing
+        path = f"zip://{path}"
+
+    path, archive, scheme = _parse_uri(path)
+
+    if scheme or archive or path.endswith(".zip"):
+        return _construct_vsi_path(path, archive, scheme)
+
+    return path
+
+
+# Supported URI schemes and their mapping to GDAL's VSI suffix.
+SCHEMES = {
+    "file": "file",
+    "zip": "zip",
+    "tar": "tar",
+    "gzip": "gzip",
+    "http": "curl",
+    "https": "curl",
+    "ftp": "curl",
+    "s3": "s3",
+    "gs": "gs",
+    "az": "az",
+    "adls": "adls",
+    "adl": "adls",  # fsspec uses this
+    "hdfs": "hdfs",
+    "webhdfs": "webhdfs",
+    # GDAL additionally supports oss and swift for remote filesystems, but
+    # those are for now not added as supported URI
+}
+
+CURLSCHEMES = {k for k, v in SCHEMES.items() if v == "curl"}
+
+
+def _parse_uri(path: str):
+    """
+    Parse a URI
+
+    Returns a tuples of (path, archive, scheme)
+
+    path : str
+        Parsed path. Includes the hostname and query string in the case
+        of a URI.
+    archive : str
+        Parsed archive path.
+    scheme : str
+        URI scheme such as "https" or "zip+s3".
+    """
+    parts = urlparse(path)
+
+    # if the scheme is not one of GDAL's supported schemes, return raw path
+    if parts.scheme and not all(p in SCHEMES for p in parts.scheme.split("+")):
+        return path, "", ""
+
+    # we have a URI
+    path = parts.path
+    scheme = parts.scheme or ""
+
+    if parts.query:
+        path += "?" + parts.query
+
+    if parts.scheme and parts.netloc:
+        path = parts.netloc + path
+
+    parts = path.split("!")
+    path = parts.pop() if parts else ""
+    archive = parts.pop() if parts else ""
+    return (path, archive, scheme)
+
+
+def _construct_vsi_path(path, archive, scheme) -> str:
+    """Convert a parsed path to a GDAL VSI path"""
+
+    prefix = ""
+    suffix = ""
+    schemes = scheme.split("+")
+
+    if "zip" not in schemes and (archive.endswith(".zip") or path.endswith(".zip")):
+        schemes.insert(0, "zip")
+
+    if schemes:
+        prefix = "/".join(
+            "vsi{0}".format(SCHEMES[p]) for p in schemes if p and p != "file"
+        )
+
+        if schemes[-1] in CURLSCHEMES:
+            suffix = f"{schemes[-1]}://"
+
+    if prefix:
+        if archive:
+            return "/{}/{}{}/{}".format(prefix, suffix, archive, path.lstrip("/"))
+        else:
+            return "/{}/{}{}".format(prefix, suffix, path)
+
+    return path


Test patch:
diff --git a/geopandas/io/tests/test_file.py b/geopandas/io/tests/test_file.py
index 671759f4..351b2752 100644
--- a/geopandas/io/tests/test_file.py
+++ b/geopandas/io/tests/test_file.py
@@ -691,10 +691,8 @@ def test_infer_zipped_file(engine, nybb_filename):
 
     # Check that it can add a zip scheme for a path that includes a subpath
     # within the archive.
-    # TODO: fix for fiona
-    if engine == "pyogrio":
-        gdf = read_file(path + "!nybb.shp", engine=engine)
-        assert isinstance(gdf, geopandas.GeoDataFrame)
+    gdf = read_file(path + "!nybb.shp", engine=engine)
+    assert isinstance(gdf, geopandas.GeoDataFrame)
 
 
 def test_allow_legacy_gdal_path(engine, nybb_filename):