Skip to content

[Breaking] Return missing if the field is set but null. #238

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.

Sign up for GitHub

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

Merged
merged 14 commits into from
Oct 8, 2021
Merged

[Breaking] Return missing if the field is set but null. #238

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
042fab2
Return `missing` if the field is set but null.
yeesian Aug 31, 2021
e59dd31
Add docstring references for GDAL/OGR RFCs
yeesian Sep 1, 2021
f18c8c6
Document the intended ArchGDAL behavior for NULL semantics
yeesian Sep 16, 2021
bc2f038
Add tests and implementation for getfield with missing values
yeesian Sep 16, 2021
9584962
Update the implementation
yeesian Sep 16, 2021
efd22c7
Implement setting fields to missing or nothing.
yeesian Sep 16, 2021
763d839
Update the documentation to indicate the methods being discussed
yeesian Sep 16, 2021
63b6f98
remove assertions
yeesian Sep 16, 2021
08ffc27
Map unset fields to `nothing` and null values to `missing`
yeesian Sep 25, 2021
364f171
Turn references into hyperlinks
yeesian Sep 26, 2021
323cbdb
use isfieldset() and isfieldnull() when getting a field
yeesian Sep 26, 2021
5c7b677
raise an error instead of getting default if we do not recognize the …
yeesian Sep 26, 2021
092bed4
Fix docstring for isfieldsetandnotnull()
yeesian Sep 26, 2021
a7710c2
Update to reflect NA for reading unset & null fields
yeesian Sep 30, 2021
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: 1 addition & 1 deletion src/base/display.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ function Base.show(io::IO, layer::AbstractFeatureLayer)::Nothing
nfielddisplay = min(n, 5)
for i in 1:nfielddisplay
fd = getfielddefn(featuredefn, i - 1)
display = " Field $(i - 1) ($(getname(fd))): [$(gettype(fd))]"
display = " Field $(i - 1) ($(getname(fd))): [$(getfieldtype(fd))]"
if length(display) > 75
println(io, "$display[1:70]...")
continue
Expand Down
54 changes: 50 additions & 4 deletions src/ogr/feature.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,50 @@ function unsetfield!(feature::Feature, i::Integer)::Feature
return feature
end

"""
isfieldnull(feature::Feature, i::Integer)

Test if a field is null.

### Parameters
* `feature`: the feature that owned the field.
* `i`: the field to test, from 0 to GetFieldCount()-1.

### Returns
`true` if the field is null, otherwise `false`.
"""
isfieldnull(feature::Feature, i::Integer)::Bool =
Bool(GDAL.ogr_f_isfieldnull(feature.ptr, i))

"""
isfieldsetandnotnull(feature::Feature, i::Integer)

Test if a field is set and not null.

### Parameters
* `feature`: the feature that owned the field.
* `i`: the field to test, from 0 to GetFieldCount()-1.

### Returns
`true` if the field is null, otherwise `false`.
"""
isfieldsetandnotnull(feature::Feature, i::Integer)::Bool =
Bool(GDAL.ogr_f_isfieldsetandnotnull(feature.ptr, i))

"""
setfieldnull!(feature::Feature, i::Integer)

Clear a field, marking it as null.

### Parameters
* `feature`: the feature that owned the field.
* `i`: the field to set to null, from 0 to GetFieldCount()-1.
"""
function setfieldnull!(feature::Feature, i::Integer)::Feature
GDAL.ogr_f_setfieldnull(feature.ptr, i)
return feature
end

# """
# OGR_F_GetRawFieldRef(OGRFeatureH hFeat,
# int iField) -> OGRField *
Expand Down Expand Up @@ -403,12 +447,14 @@ const _FETCHFIELD = Dict{OGRFieldType,Function}(
)

function getfield(feature::Feature, i::Integer)
return if isfieldset(feature, i)
_fieldtype = gettype(getfielddefn(feature, i))
return if !isfieldset(feature, i)
getdefault(feature, i)
elseif isfieldnull(feature, i)
missing
else
_fieldtype = getfieldtype(getfielddefn(feature, i))
_fetchfield = get(_FETCHFIELD, _fieldtype, getdefault)
_fetchfield(feature, i)
else
getdefault(feature, i)
end
end

Expand Down
20 changes: 20 additions & 0 deletions src/ogr/fielddefn.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,26 @@ function setsubtype!(fielddefn::FieldDefn, subtype::OGRFieldSubType)::FieldDefn
return fielddefn
end

"""
getfieldtype(fielddefn::AbstractFieldDefn)

Returns the type or subtype (if any) of this field.

### Parameters
* `fielddefn`: handle to the field definition.

### Returns
The field type or subtype.
"""
function getfieldtype(fielddefn::AbstractFieldDefn)::Union{OGRFieldType, OGRFieldSubType}
fieldsubtype = getsubtype(fielddefn)
return if fieldsubtype != OFSTNone
fieldsubtype
else
gettype(fielddefn)
end
end

"""
getjustify(fielddefn::AbstractFieldDefn)

Expand Down
15 changes: 15 additions & 0 deletions test/test_feature.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ const AG = ArchGDAL;
AG.setfield!(feature, 9, Int16(1))
AG.setfield!(feature, 10, Int32(1))
AG.setfield!(feature, 11, Float32(1.0))
for i in 1:AG.nfield(feature)
@test !AG.isfieldnull(feature, i-1)
@test AG.isfieldsetandnotnull(feature, i-1)
end
@test sprint(print, AG.getgeom(feature)) == "NULL Geometry"
AG.getgeom(feature) do geom
@test sprint(print, geom) == "NULL Geometry"
Expand All @@ -169,6 +173,10 @@ const AG = ArchGDAL;
AG.addfeature(layer) do newfeature
AG.setfrom!(newfeature, feature)
@test AG.getfield(newfeature, 0) == 1
for i in 1:AG.nfield(newfeature)
@test !AG.isfieldnull(newfeature, i-1)
@test AG.isfieldsetandnotnull(newfeature, i-1)
end
@test AG.getfield(newfeature, 1) ≈ 1.0
@test AG.getfield(newfeature, 2) == Int32[1, 2]
@test AG.getfield(newfeature, 3) == Int64[1, 2]
Expand Down Expand Up @@ -209,6 +217,13 @@ const AG = ArchGDAL;
@test AG.getfield(newfeature, 0) == 45
@test AG.getfield(newfeature, 1) ≈ 18.2
@test AG.getfield(newfeature, 5) == String["foo", "bar"]

@test AG.isfieldsetandnotnull(newfeature, 5)
AG.setfieldnull!(newfeature, 5)
@test !AG.isfieldsetandnotnull(newfeature, 5)
@test AG.isfieldset(newfeature, 5)
@test AG.isfieldnull(newfeature, 5)
@test ismissing(AG.getfield(newfeature, 5))
end
@test AG.nfeature(layer) == 1
end
Expand Down
5 changes: 5 additions & 0 deletions test/test_fielddefn.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,19 @@ const AG = ArchGDAL;
AG.setname!(fd, "newname")
@test AG.getname(fd) == "newname"
@test AG.gettype(fd) == AG.OFTInteger
@test AG.getfieldtype(fd) == AG.OFTInteger
AG.settype!(fd, AG.OFTDate)
@test AG.gettype(fd) == AG.OFTDate
@test AG.getfieldtype(fd) == AG.OFTDate
AG.settype!(fd, AG.OFTInteger)
@test AG.getsubtype(fd) == AG.OFSTNone
@test AG.getfieldtype(fd) == ArchGDAL.OFTInteger
AG.setsubtype!(fd, AG.OFSTInt16)
@test AG.getsubtype(fd) == AG.OFSTInt16
@test AG.getfieldtype(fd) == AG.OFSTInt16
AG.setsubtype!(fd, AG.OFSTBoolean)
@test AG.getsubtype(fd) == AG.OFSTBoolean
@test AG.getfieldtype(fd) == AG.OFSTBoolean
AG.setsubtype!(fd, AG.OFSTNone)
@test AG.getjustify(fd) == AG.OJUndefined
AG.setjustify!(fd, AG.OJLeft)
Expand Down
71 changes: 31 additions & 40 deletions test/test_tables.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ using Tables
withmissingfield::Bool,
withmixedgeomtypes::Bool,
reference_geotable::NamedTuple,
)::Bool
)
map_on_test_dataset(
drvshortname,
geomfamilly;
Expand All @@ -497,18 +497,14 @@ using Tables
withmixedgeomtypes = withmixedgeomtypes,
) do ds
layer = AG.getlayer(ds, 0)
return all([
keys(Tables.columntable(layer)) ==
reference_geotable.names,
eltype.(values(Tables.columntable(layer))) ==
reference_geotable.types,
tupleoftuples_equal(
columntablevalues_toWKT(
values(Tables.columntable(layer)),
),
reference_geotable.values,
@test keys(Tables.columntable(layer)) == reference_geotable.names
@test eltype.(values(Tables.columntable(layer))) == reference_geotable.types
@test tupleoftuples_equal(
columntablevalues_toWKT(
values(Tables.columntable(layer)),
),
])
reference_geotable.values,
)
end
end

Expand All @@ -524,36 +520,33 @@ using Tables
function test_layer_to_table(
layer::AG.AbstractFeatureLayer,
reference_geotable::NamedTuple,
)::Bool
return all([
keys(Tables.columntable(layer)) == reference_geotable.names,
eltype.(values(Tables.columntable(layer))) ==
reference_geotable.types,
tupleoftuples_equal(
columntablevalues_toWKT(
values(Tables.columntable(layer)),
),
reference_geotable.values,
)
@test keys(Tables.columntable(layer)) == reference_geotable.names
@test eltype.(values(Tables.columntable(layer))) == reference_geotable.types
@test tupleoftuples_equal(
columntablevalues_toWKT(
values(Tables.columntable(layer)),
),
])
reference_geotable.values,
)
end

@testset "Conversion to table for ESRI Shapefile driver" begin
ESRI_Shapefile_test_reference_geotable = (
names = (Symbol(""), :id, :name),
types = (Union{Missing,ArchGDAL.IGeometry}, Int64, String),
types = (Union{Missing,ArchGDAL.IGeometry}, Union{Missing,Int64}, String),
values = (
Union{Missing,String}[
"POLYGON ((0 0,0 1,1 1))",
"MULTIPOLYGON (((0 0,0 1,1 1)),((0.9 0.9,0.0 0.9,0 0)))",
missing,
"POLYGON ((0 0,-1 0,-1 1))",
],
[1, 2, 3, 0],
[1, 2, 3, missing],
["polygon1", "multipolygon1", "emptygeom", "emptyid"],
),
)
@test test_layer_to_table(
test_layer_to_table(
"ESRI Shapefile",
"polygon",
true,
Expand All @@ -569,7 +562,7 @@ using Tables
Missing,
ArchGDAL.IGeometry{ArchGDAL.wkbLineString},
},
Int64,
Union{Missing,Int64},
String,
),
values = (
Expand All @@ -579,11 +572,11 @@ using Tables
missing,
"LINESTRING (5 6,6 7,7 8)",
],
[1, 2, 3, 0],
[1, 2, 3, missing],
["line1", "line2", "emptygeom", "emptyid"],
),
)
@test test_layer_to_table(
test_layer_to_table(
"ESRI Shapefile",
"line",
true,
Expand Down Expand Up @@ -612,7 +605,7 @@ using Tables
["polygon1", "multipolygon1", "emptygeom", "emptyid"],
),
)
@test test_layer_to_table(
test_layer_to_table(
"GeoJSON",
"polygon",
true,
Expand Down Expand Up @@ -642,7 +635,7 @@ using Tables
["line1", "line2", "emptygeom", "emptyid"],
),
)
@test test_layer_to_table(
test_layer_to_table(
"GeoJSON",
"line",
true,
Expand Down Expand Up @@ -678,7 +671,7 @@ using Tables
["line1", "multiline1", "emptygeom", "emptyid"],
),
)
@test test_layer_to_table(
test_layer_to_table(
"GML",
"line",
true,
Expand All @@ -691,19 +684,19 @@ using Tables
@testset "Conversion to table for GPKG driver" begin
GPKG_test_reference_geotable = (
names = (:geom, :id, :name),
types = (Union{Missing,ArchGDAL.IGeometry}, Int64, String),
types = (Union{Missing,ArchGDAL.IGeometry}, Union{Missing,Int64}, String),
values = (
Union{Missing,String}[
"LINESTRING (1 2,2 3,3 4)",
"MULTILINESTRING ((1 2,2 3,3 4,4 5),(6 7,7 8,8 9,9 10))",
missing,
"LINESTRING (5 6,6 7,7 8)",
],
[1, 2, 3, 0],
Union{Missing,Int64}[1, 2, 3, missing],
["line1", "multiline1", "emptygeom", "emptyid"],
),
)
@test test_layer_to_table(
test_layer_to_table(
"GPKG",
"line",
true,
Expand All @@ -727,7 +720,7 @@ using Tables
["", "", ""],
),
)
@test test_layer_to_table(
test_layer_to_table(
"KML",
"line",
true,
Expand All @@ -751,7 +744,7 @@ using Tables
["emptyid", "multiline1", "line1"],
),
)
@test test_layer_to_table(
test_layer_to_table(
"FlatGeobuf",
"line",
true,
Expand All @@ -762,7 +755,6 @@ using Tables
end

@testset "Conversion to table for CSV driver" begin
@test begin
AG.read(
joinpath(@__DIR__, "data/multi_geom.csv"),
options = [
Expand Down Expand Up @@ -797,11 +789,10 @@ using Tables
["Mumbai", "New Delhi"],
),
)
return test_layer_to_table(
test_layer_to_table(
multigeom_test_layer,
CSV_multigeom_test_reference_geotable,
)
end
end
end

Expand Down