From 636da70c3201562f9c25e98ed5a4093000c542e0 Mon Sep 17 00:00:00 2001 From: Sean Gillies Date: Sun, 10 Mar 2024 20:34:03 -0600 Subject: [PATCH] Allow a GeoJSON collection's name to be set in write mode Resolves #1351 --- CHANGES.txt | 2 ++ fiona/collection.py | 19 ++++++----------- fiona/ogrext.pyx | 26 +++++++++++++----------- tests/test_collection.py | 44 ++++++++++++++++++++++++---------------- 4 files changed, 49 insertions(+), 42 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 362df1bf..1e900577 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 + (#). - 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 7e2fca97..c57531fb 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 @@ def __init__( 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 @@ def writerecords(self, records): 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 c63004ae..2e88e232 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 # collection.layer or 0 # if collection.layer is not None else collection.name + + if isinstance(layername, str): + layername_b = layername.encode('utf-8') + self.cogr_layer = GDALDatasetGetLayerByName(self.cogr_ds, layername_b) + elif isinstance(layername, int): + self.cogr_layer = GDALDatasetGetLayer(self.cogr_ds, 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, layername_b, cogr_srs, geometry_code, options)) except Exception as exc: diff --git a/tests/test_collection.py b/tests/test_collection.py index b7d9d443..e684ef33 100644 --- a/tests/test_collection.py +++ b/tests/test_collection.py @@ -1,7 +1,9 @@ # Testing collections and workspaces import datetime +import json import logging +import os import random import re import sys @@ -10,8 +12,8 @@ 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 +22,7 @@ ) from fiona.model import Feature, Geometry -from .conftest import WGS84PATTERN, get_temp_filename +from .conftest import WGS84PATTERN class TestSupportedDrivers: @@ -340,7 +342,7 @@ def test_include_fields__ignore_fields_error(self): "r", include_fields=["AREA"], ignore_fields=["STATE"], - ) as collection: + ): pass def test_ignore_geometry(self): @@ -400,14 +402,6 @@ def test_filter_where(self): 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 +416,6 @@ def test_filter_bbox_where(self): 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 +1190,24 @@ def test_driver_detection(tmpdir, extension, driver): crs="EPSG:4326", ) as output: assert output.driver == driver + + +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"