Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow a GeoJSON collection's name to be set in write mode #1352

Merged
merged 5 commits into from
Mar 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 6 additions & 13 deletions fiona/collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from contextlib import ExitStack
import logging
import os
from pathlib import Path
import warnings

from fiona import compat, vfs
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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.
"""
Expand Down
26 changes: 14 additions & 12 deletions fiona/ogrext.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
51 changes: 33 additions & 18 deletions tests/test_collection.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -10,8 +13,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,
Expand All @@ -20,7 +23,7 @@
)
from fiona.model import Feature, Geometry

from .conftest import WGS84PATTERN, get_temp_filename
from .conftest import WGS84PATTERN


class TestSupportedDrivers:
Expand Down Expand Up @@ -340,7 +343,7 @@ def test_include_fields__ignore_fields_error(self):
"r",
include_fields=["AREA"],
ignore_fields=["STATE"],
) as collection:
):
pass

def test_ignore_geometry(self):
Expand Down Expand Up @@ -400,14 +403,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):
Expand All @@ -422,11 +417,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):
Expand Down Expand Up @@ -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",
)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll figure this out later. Some lack of libiconv or configuration thereof, I bet.

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,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to be sure that we support Unicode layer names 😆 ⚽ 🇺🇾 🟥

) as colxn:
assert colxn.name == "Darwin Núñez"

with open(filename) as f:
geojson = json.load(f)

assert geojson["name"] == "Darwin Núñez"
Loading