WIP: resolving #512

This work solves the problem for GDAL 1.x.

TODO: do the same for _shim2.pyx and _shim22.pyx

fiona/_csl.pxd

@@ -1,5 +1,6 @@
 # String API functions.
 
 cdef extern from "cpl_string.h":
+    char ** CSLAddNameValue (char **list, char *name, char *value)
     char ** CSLSetNameValue (char **list, char *name, char *value)
     void    CSLDestroy (char **list)

fiona/_shim1.pxd

@@ -2,8 +2,8 @@ include "ogrext1.pxd"
 
 cdef bint is_field_null(void *feature, int n)
 cdef void gdal_flush_cache(void *cogr_ds)
-cdef void* gdal_open_vector(char* path_c, int mode, drivers)
-cdef void* gdal_create(void* cogr_driver, const char *path_c) except *
+cdef void* gdal_open_vector(char* path_c, int mode, drivers, options)
+cdef void* gdal_create(void* cogr_driver, const char *path_c, options) except *
 cdef OGRErr gdal_start_transaction(void *cogr_ds, int force)
 cdef OGRErr gdal_commit_transaction(void *cogr_ds)
 cdef OGRErr gdal_rollback_transaction(void *cogr_ds)

fiona/_shim1.pyx

@@ -1,10 +1,16 @@
+import logging
+
 from fiona.ogrext1 cimport *
 
 from fiona.errors import DriverIOError
 from fiona._err cimport exc_wrap_pointer
 
+
+log = logging.getLogger(__name__)
+
 cdef int OGRERR_NONE = 0
 
+
 cdef bint is_field_null(void *feature, int n):
     if not OGR_F_IsFieldSet(feature, n):
         return True
@@ -16,10 +22,18 @@ cdef void gdal_flush_cache(void *cogr_ds):
     if retval != OGRERR_NONE:
         raise RuntimeError("Failed to sync to disk")
 
-cdef void* gdal_open_vector(const char *path_c, int mode, drivers):
+cdef void* gdal_open_vector(const char *path_c, int mode, drivers, options):
     cdef void* cogr_ds = NULL
     cdef void* drv = NULL
     cdef void* ds = NULL
+
+    encoding = options.get('encoding', None)
+    if encoding:
+        val = encoding.encode('utf-8')
+        CPLSetThreadLocalConfigOption('SHAPE_ENCODING', <const char *>val)
+    else:
+        CPLSetThreadLocalConfigOption('SHAPE_ENCODING', "")
+
     if drivers:
         for name in drivers:
             name_b = name.encode()
@@ -37,15 +51,35 @@ cdef void* gdal_open_vector(const char *path_c, int mode, drivers):
         cogr_ds = OGROpen(path_c, mode, NULL)
     return cogr_ds
 
-cdef void* gdal_create(void* cogr_driver, const char *path_c) except *:
-    cdef void* cogr_ds
+cdef void* gdal_create(void* cogr_driver, const char *path_c, options) except *:
+    cdef void* cogr_ds = NULL
+    cdef char **opts = NULL
+
+    encoding = options.get('encoding', None)
+    if encoding:
+        val = encoding.encode('utf-8')
+        CPLSetThreadLocalConfigOption('SHAPE_ENCODING', val)
+    else:
+        CPLSetThreadLocalConfigOption('SHAPE_ENCODING', "")
+
+    for k, v in options.items():
+        k = k.upper().encode('utf-8')
+        if isinstance(v, bool):
+            v = ('ON' if v else 'OFF').encode('utf-8')
+        else:
+            v = str(v).encode('utf-8')
+        log.debug("Set option %r: %r", k, v)
+        opts = CSLAddNameValue(opts, <const char *>k, <const char *>v)
+
     try:
         cogr_ds = exc_wrap_pointer(
             OGR_Dr_CreateDataSource(
-                cogr_driver, path_c, NULL))
+                cogr_driver, path_c, opts))
+        return cogr_ds
     except Exception as exc:
         raise DriverIOError(str(exc))
-    return cogr_ds
+    finally:
+        CSLDestroy(opts)
 
 # transactions are not supported in GDAL 1.x
 cdef OGRErr gdal_start_transaction(void* cogr_ds, int force):

fiona/collection.py

@@ -145,12 +145,13 @@ def __init__(self, path, mode='r', driver=None, schema=None, crs=None,
         self.env.__enter__()
 
         self._driver = driver
+        kwargs.update(encoding=encoding or '')
         self.encoding = encoding
 
         try:
             if self.mode == 'r':
                 self.session = Session()
-                self.session.start(self)
+                self.session.start(self, **kwargs)
             elif self.mode in ('a', 'w'):
                 self.session = WritingSession()
                 self.session.start(self, **kwargs)

fiona/ogrext.pyx

@@ -395,7 +395,7 @@ cdef class Session:
     def __dealloc__(self):
         self.stop()
 
-    def start(self, collection):
+    def start(self, collection, **kwargs):
         cdef const char *path_c = NULL
         cdef const char *name_c = NULL
         cdef void *drv = NULL
@@ -413,6 +413,8 @@ cdef class Session:
             path_b = path
         path_c = path_b
 
+        userencoding = kwargs.get('encoding')
+
         # TODO: eliminate this context manager in 2.0 as we have done
         # in Rasterio 1.0.
         with cpl_errs:
@@ -426,7 +428,7 @@ cdef class Session:
             else:
                 drivers = None
 
-            self.cogr_ds = gdal_open_vector(path_c, 0, drivers)
+            self.cogr_ds = gdal_open_vector(path_c, 0, drivers, kwargs)
 
         if self.cogr_ds == NULL:
             raise FionaValueError(
@@ -449,19 +451,14 @@ cdef class Session:
         if self.cogr_layer == NULL:
             raise ValueError("Null layer: " + repr(collection.name))
 
-        self.collection = collection
+        self._fileencoding = userencoding or (
+            OGR_L_TestCapability(
+                self.cogr_layer, OLC_STRINGSASUTF8) and
+            'utf-8') or (
+            self.get_driver() == "ESRI Shapefile" and
+            'ISO-8859-1') or locale.getpreferredencoding().upper()
 
-        userencoding = self.collection.encoding
-        if userencoding:
-            CPLSetThreadLocalConfigOption('SHAPE_ENCODING', '')
-            self._fileencoding = userencoding.upper()
-        else:
-            self._fileencoding = (
-                OGR_L_TestCapability(
-                    self.cogr_layer, OLC_STRINGSASUTF8) and
-                'utf-8') or (
-                self.get_driver() == "ESRI Shapefile" and
-                'ISO-8859-1') or locale.getpreferredencoding().upper()
+        self.collection = collection
 
     def stop(self):
         self.cogr_layer = NULL
@@ -712,13 +709,15 @@ cdef class WritingSession(Session):
     def start(self, collection, **kwargs):
         cdef void *cogr_srs = NULL
         cdef char **options = NULL
-        self.collection = collection
         cdef const char *path_c = NULL
         cdef const char *driver_c = NULL
         cdef const char *name_c = NULL
         cdef const char *proj_c = NULL
         cdef const char *fileencoding_c = NULL
         path = collection.path
+        self.collection = collection
+
+        userencoding = kwargs.get('encoding')
 
         if collection.mode == 'a':
             if os.path.exists(path):
@@ -727,7 +726,7 @@ cdef class WritingSession(Session):
                 except UnicodeDecodeError:
                     path_b = path
                 path_c = path_b
-                self.cogr_ds = gdal_open_vector(path_c, 1, None)
+                self.cogr_ds = gdal_open_vector(path_c, 1, None, kwargs)
 
                 cogr_driver = GDALGetDatasetDriver(self.cogr_ds)
                 if cogr_driver == NULL:
@@ -748,7 +747,6 @@ cdef class WritingSession(Session):
             else:
                 raise OSError("No such file or directory %s" % path)
 
-            userencoding = self.collection.encoding
             self._fileencoding = (userencoding or (
                 OGR_L_TestCapability(self.cogr_layer, OLC_STRINGSASUTF8) and
                 'utf-8') or (
@@ -765,15 +763,6 @@ cdef class WritingSession(Session):
             driver_b = collection.driver.encode()
             driver_c = driver_b
 
-            # Creation options
-            for k, v in kwargs.items():
-                k, v = k.upper(), str(v).upper()
-                key_b = k.encode('utf-8')
-                val_b = v.encode('utf-8')
-                key_c = key_b
-                val_c = val_b
-                options = CSLSetNameValue(options, key_c, val_c)
-                log.debug("Option %r=%r\n", k, v)
 
             cogr_driver = GDALGetDriverByName(driver_c)
             if cogr_driver == NULL:
@@ -785,23 +774,23 @@ cdef class WritingSession(Session):
             #
             # TODO: remove the assumption.
             if not os.path.exists(path):
-                cogr_ds = exc_wrap_pointer(gdal_create(cogr_driver, path_c))
+                cogr_ds = exc_wrap_pointer(gdal_create(cogr_driver, path_c, kwargs))
 
             # TODO: revisit the logic in the following blocks when we
             # change the assumption above.
             # TODO: use exc_wrap_pointer()
             else:
-                cogr_ds = gdal_open_vector(path_c, 1, None)
+                cogr_ds = gdal_open_vector(path_c, 1, None, kwargs)
 
                 # TODO: use exc_wrap_pointer()
                 if cogr_ds == NULL:
-                    cogr_ds = gdal_create(cogr_driver, path_c)
+                    cogr_ds = gdal_create(cogr_driver, path_c, kwargs)
 
                 elif collection.name is None:
                     GDALClose(cogr_ds)
                     cogr_ds = NULL
                     log.debug("Deleted pre-existing data at %s", path)
-                    cogr_ds = gdal_create(cogr_driver, path_c)
+                    cogr_ds = gdal_create(cogr_driver, path_c, kwargs)
 
                 else:
                     pass
@@ -857,7 +846,6 @@ cdef class WritingSession(Session):
             # to the collection constructor takes highest precedence, then
             # 'iso-8859-1', then the system's default encoding as last resort.
             sysencoding = locale.getpreferredencoding()
-            userencoding = collection.encoding
             self._fileencoding = (userencoding or (
                 collection.driver == "ESRI Shapefile" and
                 'ISO-8859-1') or sysencoding).upper()
@@ -866,7 +854,7 @@ cdef class WritingSession(Session):
             # will result in a warning. Fixing is a TODO.
             fileencoding = self.get_fileencoding()
             if fileencoding:
-                fileencoding_b = fileencoding.encode()
+                fileencoding_b = fileencoding.encode('utf-8')
                 fileencoding_c = fileencoding_b
                 with cpl_errs:
                     options = CSLSetNameValue(options, "ENCODING", fileencoding_c)
@@ -895,6 +883,15 @@ cdef class WritingSession(Session):
             name_b = collection.name.encode('utf-8')
             name_c = name_b
 
+            for k, v in kwargs.items():
+                k = k.upper().encode('utf-8')
+                if isinstance(v, bool):
+                    v = ('ON' if v else 'OFF').encode('utf-8')
+                else:
+                    v = str(v).encode('utf-8')
+                log.debug("Set option %r: %r", k, v)
+                options = CSLAddNameValue(options, <const char *>k, <const char *>v)
+
             try:
                 self.cogr_layer = exc_wrap_pointer(
                     GDALDatasetCreateLayer(
@@ -1275,7 +1272,7 @@ def _remove(path, driver=None):
         raise RuntimeError("Failed to remove data source {}".format(path))
 
 
-def _listlayers(path):
+def _listlayers(path, **kwargs):
 
     """Provides a list of the layers in an OGR data source.
     """
@@ -1292,7 +1289,7 @@ def _listlayers(path):
         path_b = path
     path_c = path_b
     with cpl_errs:
-        cogr_ds = gdal_open_vector(path_c, 0, None)
+        cogr_ds = gdal_open_vector(path_c, 0, None, kwargs)
     if cogr_ds == NULL:
         raise ValueError("No data available at path '%s'" % path)
 

fiona/ogrext1.pxd

@@ -16,6 +16,7 @@ cdef extern from "cpl_conv.h":
     const char *CPLGetConfigOption (char *, char *)
 
 cdef extern from "cpl_string.h":
+    char ** CSLAddNameValue (char **list, char *name, char *value)
     char ** CSLSetNameValue (char **list, char *name, char *value)
     void    CSLDestroy (char **list)
 

tests/test_fio_ls.py

@@ -18,7 +18,7 @@ def test_fio_ls_single_layer():
         DATA_DIR])
     assert result.exit_code == 0
     assert len(result.output.splitlines()) == 1
-    assert json.loads(result.output) == ['coutwildrnp']
+    assert json.loads(result.output) == ['coutwildrnp', 'gre']
 
 
 def test_fio_ls_indent(path_coutwildrnp_shp):
@@ -50,7 +50,7 @@ def test_fio_ls_multi_layer(path_coutwildrnp_shp, tmpdir):
         'ls', outdir])
     assert result.exit_code == 0
     json_result = json.loads(result.output)
-    assert sorted(json_result)  == sorted(layer_names)
+    assert sorted(json_result) == sorted(layer_names)
 
 
 def test_fio_ls_vfs(path_coutwildrnp_zip):

tests/test_listing.py

@@ -22,11 +22,11 @@ def test_single_file(path_coutwildrnp_shp):
 
 
 def test_directory(data_dir):
-    assert fiona.listlayers(data_dir) == ['coutwildrnp']
+    assert fiona.listlayers(data_dir) == ['coutwildrnp', 'gre']
 
 
 def test_directory_trailing_slash(data_dir):
-    assert fiona.listlayers(data_dir) == ['coutwildrnp']
+    assert fiona.listlayers(data_dir) == ['coutwildrnp', 'gre']
 
 
 def test_zip_path(path_coutwildrnp_zip):