Repository: Toblerity/Fiona (python)

Issue:
Add name keyword argument to fiona.open() and Collection constructor
And pass it through to GDALDatasetCreateLayer().

See also https://github.com/Toblerity/Fiona/discussions/1348.

Pull request description:
Allow a GeoJSON collection's name to be set in write mode
Resolves #1351

Patch:
diff --git a/CHANGES.txt b/CHANGES.txt
index 362df1b..2962954 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -8,6 +8,8 @@ All issue numbers are relative to https://github.com/Toblerity/Fiona/issues.
 
 Bug fixes:
 
+- Allow a GeoJSON collection's layer name to be set on opening in write mode
+  (#1352).
 - The legacy crs.py module which was shadowed by the new crs.pyx module has
   been deleted (#1344).
 - Python 3.8 has been added back to the list of supported versions and
diff --git a/fiona/collection.py b/fiona/collection.py
index 7e2fca9..c57531f 100644
--- a/fiona/collection.py
+++ b/fiona/collection.py
@@ -2,7 +2,7 @@
 
 from contextlib import ExitStack
 import logging
-import os
+from pathlib import Path
 import warnings
 
 from fiona import compat, vfs
@@ -173,21 +173,14 @@ class Collection:
             path = _parse_path(path)
             self.path = _vsi_path(path)
 
+        self.layer = layer or 0
+
         if mode == "w":
             if layer and not isinstance(layer, str):
                 raise ValueError("in 'w' mode, layer names must be strings")
-            if driver == "GeoJSON":
-                if layer is not None:
-                    raise ValueError("the GeoJSON format does not have layers")
-                self.name = "OgrGeoJSON"
-            # TODO: raise ValueError as above for other single-layer formats.
-            else:
-                self.name = layer or os.path.basename(os.path.splitext(path.path)[0])
+            self.name = layer or Path(self.path).stem
         else:
-            if layer in (0, None):
-                self.name = 0
-            else:
-                self.name = layer or os.path.basename(os.path.splitext(path)[0])
+            self.name = 0 if layer is None else layer or Path(self.path).stem
 
         self.mode = mode
 
@@ -551,7 +544,7 @@ class Collection:
 
     def write(self, record):
         """Stages a record for writing to disk.
-        
+
         Note: Each call of this method will start and commit a
         unique transaction with the data source.
         """
diff --git a/fiona/ogrext.pyx b/fiona/ogrext.pyx
index c63004a..f73fedd 100644
--- a/fiona/ogrext.pyx
+++ b/fiona/ogrext.pyx
@@ -587,19 +587,21 @@ cdef class Session:
 
         self.cogr_ds = gdal_open_vector(path_c, 0, drivers, kwargs)
 
-        if isinstance(collection.name, str):
-            name_b = collection.name.encode('utf-8')
-            name_c = name_b
-            self.cogr_layer = GDALDatasetGetLayerByName(self.cogr_ds, name_c)
-        elif isinstance(collection.name, int):
-            self.cogr_layer = GDALDatasetGetLayer(self.cogr_ds, collection.name)
+        layername = collection.name
+
+        if isinstance(layername, str):
+            layername_b = layername.encode('utf-8')
+            self.cogr_layer = GDALDatasetGetLayerByName(self.cogr_ds, <const char *>layername_b)
+        elif isinstance(layername, int):
+            self.cogr_layer = GDALDatasetGetLayer(self.cogr_ds, <int>layername)
+
+        if self.cogr_layer == NULL:
+            raise ValueError(f"Null layer: {collection} {layername}")
+        else:
             name_c = OGR_L_GetName(self.cogr_layer)
             name_b = name_c
             collection.name = name_b.decode('utf-8')
 
-        if self.cogr_layer == NULL:
-            raise ValueError("Null layer: " + repr(collection.name))
-
         encoding = self._get_internal_encoding()
 
         if collection.ignore_fields or collection.include_fields is not None:
@@ -1196,8 +1198,8 @@ cdef class WritingSession(Session):
                 GDALDatasetDeleteLayer(self.cogr_ds, idx)
 
             # Create the named layer in the datasource.
-            name_b = collection.name.encode('utf-8')
-            name_c = name_b
+            layername = collection.name
+            layername_b = layername.encode('utf-8')
 
             # To avoid circular import.
             from fiona import meta
@@ -1239,7 +1241,7 @@ cdef class WritingSession(Session):
                 with Env(GDAL_VALIDATE_CREATION_OPTIONS="NO"):
                     self.cogr_layer = exc_wrap_pointer(
                         GDALDatasetCreateLayer(
-                            self.cogr_ds, name_c, cogr_srs,
+                            self.cogr_ds, <const char *>layername_b, cogr_srs,
                             <OGRwkbGeometryType>geometry_code, options))
 
             except Exception as exc:


Test patch:
diff --git a/tests/test_collection.py b/tests/test_collection.py
index b7d9d44..d8f5cdd 100644
--- a/tests/test_collection.py
+++ b/tests/test_collection.py
@@ -1,7 +1,10 @@
-# Testing collections and workspaces
+"""Testing collections and workspaces."""
+# coding=utf-8
 
 import datetime
+import json
 import logging
+import os
 import random
 import re
 import sys
@@ -10,8 +13,8 @@ import pytest
 
 import fiona
 from fiona.collection import Collection
-from fiona.drvsupport import supported_drivers, driver_mode_mingdal
-from fiona.env import getenv, GDALVersion
+from fiona.drvsupport import supported_drivers
+from fiona.env import getenv
 from fiona.errors import (
     AttributeFilterError,
     FionaValueError,
@@ -20,7 +23,7 @@ from fiona.errors import (
 )
 from fiona.model import Feature, Geometry
 
-from .conftest import WGS84PATTERN, get_temp_filename
+from .conftest import WGS84PATTERN
 
 
 class TestSupportedDrivers:
@@ -340,7 +343,7 @@ class TestIgnoreFieldsAndGeometry:
                 "r",
                 include_fields=["AREA"],
                 ignore_fields=["STATE"],
-            ) as collection:
+            ):
                 pass
 
     def test_ignore_geometry(self):
@@ -400,14 +403,6 @@ class TestFilterReading:
         results = list(self.c.filter())
         assert len(results) == 67
 
-    def test_filter_bbox_where(self):
-        # combined filter criteria
-        results = set(self.c.keys(
-            bbox=(-120.0, 40.0, -100.0, 50.0), where="NAME LIKE 'Mount%'"))
-        assert results == set([0, 2, 5, 13])
-        results = set(self.c.keys())
-        assert len(results) == 67
-
     def test_filter_where_error(self):
         for w in ["bad stuff", "NAME=3", "NNAME LIKE 'Mount%'"]:
             with pytest.raises(AttributeFilterError):
@@ -422,11 +417,6 @@ class TestFilterReading:
         results = set(self.c.keys())
         assert len(results) == 67
 
-    def test_filter_where_error(self):
-        for w in ["bad stuff", "NAME=3", "NNAME LIKE 'Mount%'"]:
-            with pytest.raises(AttributeFilterError):
-                self.c.filter(where=w)
-
 
 class TestUnsupportedDriver:
     def test_immediate_fail_driver(self, tmpdir):
@@ -1201,3 +1191,28 @@ def test_driver_detection(tmpdir, extension, driver):
         crs="EPSG:4326",
     ) as output:
         assert output.driver == driver
+
+
+@pytest.mark.skipif(
+    sys.platform == "win32",
+    reason="Windows test runners don't have full unicode support",
+)
+def test_collection_name(tmp_path):
+    """A Collection name is plumbed all the way through."""
+    filename = os.fspath(tmp_path.joinpath("test.geojson"))
+
+    with fiona.Collection(
+        filename,
+        "w",
+        driver="GeoJSON",
+        crs="EPSG:4326",
+        schema={"geometry": "Point", "properties": {"foo": "int"}},
+        layer="Darwin Núñez",
+        write_name=True,
+    ) as colxn:
+        assert colxn.name == "Darwin Núñez"
+
+    with open(filename) as f:
+        geojson = json.load(f)
+
+    assert geojson["name"] == "Darwin Núñez"