From 1ce26cd92567df8a60c788cb31d037c803a74548 Mon Sep 17 00:00:00 2001 From: Sean Gillies Date: Tue, 12 Mar 2024 14:07:48 -0600 Subject: [PATCH 1/3] Add an "int16" property type Resolves #1321 --- CHANGES.txt | 2 ++ fiona/ogrext.pyx | 33 +++++++++++++------ fiona/schema.pyx | 2 ++ tests/test_bytescollection.py | 2 +- tests/test_subtypes.py | 60 +++++++++++++++++++++++++---------- 5 files changed, 73 insertions(+), 26 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 2962954b6..1757a718c 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: +- Add a 16-bit integer type "int16" based on OGR's OSFTInt16 integer sub-type + (#). - 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 diff --git a/fiona/ogrext.pyx b/fiona/ogrext.pyx index f73fedd2a..6f8412289 100644 --- a/fiona/ogrext.pyx +++ b/fiona/ogrext.pyx @@ -778,6 +778,7 @@ cdef class Session: continue fieldtypename = FIELD_TYPES[OGR_Fld_GetType(cogr_fielddefn)] + fieldsubtype = OGR_Fld_GetSubType(cogr_fielddefn) if not fieldtypename: log.warning( @@ -802,14 +803,25 @@ cdef class Session: val = "float" + fmt - elif fieldtypename in ('int32', 'int64'): + elif fieldtypename == "int32": + log.debug("Checking field type name: fieldtypename=%r, fieldsubtype=%r, OFSTInt16=%r", fieldtypename, fieldsubtype, OFSTInt16) + if fieldsubtype == OFSTBoolean: + val = "bool" + elif fieldsubtype == OFSTInt16: + val = "int16" + else: + fmt = "" + width = OGR_Fld_GetWidth(cogr_fielddefn) + if width: + fmt = f":{width:d}" + val = fieldtypename + fmt + + elif fieldtypename == "int64": fmt = "" width = OGR_Fld_GetWidth(cogr_fielddefn) - if width: fmt = f":{width:d}" - - val = 'int' + fmt + val = "int" + fmt elif fieldtypename == 'str': fmt = "" @@ -1287,15 +1299,17 @@ cdef class WritingSession(Session): # Convert 'long' to 'int'. See # https://github.com/Toblerity/Fiona/issues/101. - if fiona.gdal_version.major >= 2 and value in ('int', 'long'): + if value in ('int', 'long'): value = 'int64' - elif value == 'int': - value = 'int32' - if value == 'bool': + elif value == 'bool': value = 'int32' field_subtype = OFSTBoolean + elif value == 'int16': + value = 'int32' + field_subtype = OFSTInt16 + # Is there a field width/precision? width = precision = None if ':' in value: @@ -1307,7 +1321,7 @@ cdef class WritingSession(Session): width = int(fmt) if value == 'int': - if GDAL_VERSION_NUM >= 2000000 and (width == 0 or width >= 10): + if width == 0 or width >= 10: value = 'int64' else: value = 'int32' @@ -1324,6 +1338,7 @@ cdef class WritingSession(Session): if field_subtype != OFSTNone: # subtypes are new in GDAL 2.x, ignored in 1.x OGR_Fld_SetSubType(cogr_fielddefn, field_subtype) + exc_wrap_int(OGR_L_CreateField(self.cogr_layer, cogr_fielddefn, 1)) except (UnicodeEncodeError, CPLE_BaseError) as exc: diff --git a/fiona/schema.pyx b/fiona/schema.pyx index 75e844fbf..3ea0f3f9a 100644 --- a/fiona/schema.pyx +++ b/fiona/schema.pyx @@ -74,6 +74,8 @@ def normalize_field_type(ftype): return ftype elif ftype == 'bool': return 'bool' + elif ftype == "int16": + return 'int32' elif ftype.startswith('int'): width = int((ftype.split(':')[1:] or ['0'])[0]) if GDAL_VERSION_NUM >= 2000000 and (width == 0 or width >= 10): diff --git a/tests/test_bytescollection.py b/tests/test_bytescollection.py index fdcc4f3be..3b42b85f3 100644 --- a/tests/test_bytescollection.py +++ b/tests/test_bytescollection.py @@ -93,7 +93,7 @@ def test_schema(self): assert s["NAME"] == "str" assert s["URL"] == "str" assert s["STATE_FIPS"] == "str" - assert s["WILDRNP020"] == "int" + assert s["WILDRNP020"] == "int32" def test_closed_schema(self): # Schema is lazy too, never computed in this case. TODO? diff --git a/tests/test_subtypes.py b/tests/test_subtypes.py index 8a99c5dd9..12e0e9ffb 100644 --- a/tests/test_subtypes.py +++ b/tests/test_subtypes.py @@ -1,26 +1,28 @@ +"""Tests of schema sub-types.""" + +import os + import fiona from fiona.model import Feature -def test_read_bool_subtype(tmpdir): +def test_read_bool_subtype(tmp_path): test_data = """{"type": "FeatureCollection", "features": [{"type": "Feature", "properties": {"bool": true, "not_bool": 1, "float": 42.5}, "geometry": null}]}""" - path = tmpdir.join("test_read_bool_subtype.geojson") - with open(str(path), "w") as f: + path = tmp_path.joinpath("test_read_bool_subtype.geojson") + + with open(os.fspath(path), "w") as f: f.write(test_data) - with fiona.open(str(path), "r") as src: + with fiona.open(path, "r") as src: feature = next(iter(src)) - if fiona.gdal_version.major >= 2: - assert type(feature["properties"]["bool"]) is bool - else: - assert type(feature["properties"]["bool"]) is int + assert type(feature["properties"]["bool"]) is bool assert isinstance(feature["properties"]["not_bool"], int) assert type(feature["properties"]["float"]) is float -def test_write_bool_subtype(tmpdir): - path = tmpdir.join("test_write_bool_subtype.geojson") +def test_write_bool_subtype(tmp_path): + path = tmp_path.joinpath("test_write_bool_subtype.geojson") schema = { "geometry": "Point", @@ -42,14 +44,40 @@ def test_write_bool_subtype(tmpdir): } ) - with fiona.open(str(path), "w", driver="GeoJSON", schema=schema) as dst: + with fiona.open(path, "w", driver="GeoJSON", schema=schema) as dst: dst.write(feature) - with open(str(path)) as f: + with open(os.fspath(path)) as f: data = f.read() - if fiona.gdal_version.major >= 2: - assert """"bool": true""" in data - else: - assert """"bool": 1""" in data + assert """"bool": true""" in data assert """"not_bool": 1""" in data + + +def test_write_int16_subtype(tmp_path): + path = tmp_path.joinpath("test_write_bool_subtype.gpkg") + + schema = { + "geometry": "Point", + "properties": { + "a": "int", + "b": "int16", + }, + } + + feature = Feature.from_dict( + **{ + "geometry": None, + "properties": { + "a": 1, + "b": 2, + }, + } + ) + + with fiona.open(path, "w", driver="GPKG", schema=schema) as colxn: + colxn.write(feature) + + with fiona.open(path) as colxn: + assert colxn.schema["properties"]["a"] == "int" + assert colxn.schema["properties"]["b"] == "int16" From 13b65c0c5673f4830c4d251f8df9f7655632d46f Mon Sep 17 00:00:00 2001 From: Sean Gillies Date: Tue, 12 Mar 2024 14:10:27 -0600 Subject: [PATCH 2/3] Update reference in change log --- CHANGES.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.txt b/CHANGES.txt index 1757a718c..3851329e1 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -9,7 +9,7 @@ All issue numbers are relative to https://github.com/Toblerity/Fiona/issues. Bug fixes: - Add a 16-bit integer type "int16" based on OGR's OSFTInt16 integer sub-type - (#). + (#1358). - 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 From 32277d012f3ff0c61c2ff2112e91f26f8d31b9c1 Mon Sep 17 00:00:00 2001 From: Sean Gillies Date: Wed, 13 Mar 2024 10:46:28 -0600 Subject: [PATCH 3/3] Code cleanup to improve readability --- fiona/ogrext.pyx | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/fiona/ogrext.pyx b/fiona/ogrext.pyx index 6f8412289..629c9feaa 100644 --- a/fiona/ogrext.pyx +++ b/fiona/ogrext.pyx @@ -787,24 +787,17 @@ cdef class Session: OGR_Fld_GetType(cogr_fielddefn)) continue - val = fieldtypename - - if fieldtypename == 'float': - fmt = "" + if fieldtypename == "float": width = OGR_Fld_GetWidth(cogr_fielddefn) - - if width: # and width != 24: - fmt = f":{width:d}" - precision = OGR_Fld_GetPrecision(cogr_fielddefn) - - if precision: # and precision != 15: + fmt = "" + if width: + fmt = f":{width:d}" + if precision: fmt += f".{precision:d}" - - val = "float" + fmt + val = f"float{fmt}" elif fieldtypename == "int32": - log.debug("Checking field type name: fieldtypename=%r, fieldsubtype=%r, OFSTInt16=%r", fieldtypename, fieldsubtype, OFSTInt16) if fieldsubtype == OFSTBoolean: val = "bool" elif fieldsubtype == OFSTInt16: @@ -814,23 +807,24 @@ cdef class Session: width = OGR_Fld_GetWidth(cogr_fielddefn) if width: fmt = f":{width:d}" - val = fieldtypename + fmt + val = f"int32{fmt}" elif fieldtypename == "int64": fmt = "" width = OGR_Fld_GetWidth(cogr_fielddefn) if width: fmt = f":{width:d}" - val = "int" + fmt + val = f"int{fmt}" - elif fieldtypename == 'str': + elif fieldtypename == "str": fmt = "" width = OGR_Fld_GetWidth(cogr_fielddefn) - if width: fmt = f":{width:d}" + val = f"str{fmt}" - val = fieldtypename + fmt + else: + val = fieldtypename # Store the field name and description value. props[key] = val