Skip to content

Commit df99744

Browse files
authored
gdal vector check-geometry: support --include-field ALL
Harmonizes behavior between gdal vector check-geometry and gdal raster pixel-info. Changes the gdal raster pixel-info behavior such that fields are added to the output layer in the order they are specified, not the order they occur in the source layer.
1 parent a7b8cdd commit df99744

File tree

6 files changed

+104
-46
lines changed

6 files changed

+104
-46
lines changed

apps/gdalalg_raster_pixel_info.cpp

Lines changed: 5 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ bool GDALRasterPixelInfoAlgorithm::RunStep(GDALPipelineStepRunContext &)
270270
const bool bIsGeoJSON = EQUAL(m_format.c_str(), "GeoJSON");
271271

272272
OGRLayer *poSrcLayer = nullptr;
273-
std::set<int> anSrcFieldIndicesToInclude;
273+
std::vector<int> anSrcFieldIndicesToInclude;
274274
std::vector<int> anMapSrcToDstFields;
275275

276276
if (poVectorSrcDS)
@@ -315,29 +315,11 @@ bool GDALRasterPixelInfoAlgorithm::RunStep(GDALPipelineStepRunContext &)
315315
poVectorSrcDS->GetDescription());
316316
return false;
317317
}
318-
if (m_includeFields.size() == 1 && m_includeFields[0] == "ALL")
319-
{
320-
const int nSrcFieldCount =
321-
poSrcLayer->GetLayerDefn()->GetFieldCount();
322-
for (int i = 0; i < nSrcFieldCount; ++i)
323-
anSrcFieldIndicesToInclude.insert(i);
324-
}
325-
else if (!m_includeFields.empty() &&
326-
!(m_includeFields.size() == 1 && m_includeFields[0] == "NONE"))
318+
319+
if (!GetFieldIndices(m_includeFields, OGRLayer::ToHandle(poSrcLayer),
320+
anSrcFieldIndicesToInclude))
327321
{
328-
for (const std::string &osFieldName : m_includeFields)
329-
{
330-
const int nIdx = poSrcLayer->GetLayerDefn()->GetFieldIndex(
331-
osFieldName.c_str());
332-
if (nIdx < 0)
333-
{
334-
ReportError(CE_Failure, CPLE_AppDefined,
335-
"Field '%s' does not exist in layer '%s'",
336-
osFieldName.c_str(), poSrcLayer->GetName());
337-
return false;
338-
}
339-
anSrcFieldIndicesToInclude.insert(nIdx);
340-
}
322+
return false;
341323
}
342324

343325
if (m_posCrs.empty())

apps/gdalalg_vector_check_geometry.cpp

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,10 @@ GDALVectorCheckGeometryAlgorithm::GDALVectorCheckGeometryAlgorithm(
3232
standaloneStep)
3333
{
3434
AddArg("include-field", 0,
35-
_("Fields from input layer to include in output"), &m_includeFields);
35+
_("Fields from input layer to include in output (special values: "
36+
"ALL and NONE)"),
37+
&m_includeFields)
38+
.SetDefault("NONE");
3639

3740
AddArg("include-valid", 0,
3841
_("Include valid inputs in output, with empty geometry"),
@@ -348,19 +351,11 @@ bool GDALVectorCheckGeometryAlgorithm::RunStep(GDALPipelineStepRunContext &)
348351
}
349352

350353
std::vector<int> includeFieldIndices;
351-
for (const auto &fieldName : m_includeFields)
354+
if (!GetFieldIndices(m_includeFields,
355+
OGRLayer::ToHandle(poSrcLayer),
356+
includeFieldIndices))
352357
{
353-
auto iSrcField =
354-
poSrcLayerDefn->GetFieldIndex(fieldName.c_str());
355-
if (iSrcField == -1)
356-
{
357-
ReportError(
358-
CE_Failure, CPLE_AppDefined,
359-
"Specified field '%s' does not exist in layer '%s'",
360-
fieldName.c_str(), poSrcLayer->GetDescription());
361-
return false;
362-
}
363-
includeFieldIndices.push_back(iSrcField);
358+
return false;
364359
}
365360

366361
outDS->AddLayer(*poSrcLayer,

autotest/utilities/test_gdalalg_vector_check_geometry.py

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,15 @@ def test_gdalalg_vector_check_geometry(alg, polys):
6464

6565
dst_ds = alg["output"].GetDataset()
6666
dst_lyr = dst_ds.GetLayer(0)
67+
dst_defn = dst_lyr.GetLayerDefn()
68+
6769
assert dst_lyr.GetName() == "error_location"
68-
assert dst_lyr.GetLayerDefn().GetGeomType() == ogr.wkbMultiPoint
70+
assert dst_defn.GetGeomType() == ogr.wkbMultiPoint
71+
72+
field_names = [
73+
dst_defn.GetFieldDefn(i).GetName() for i in range(dst_defn.GetFieldCount())
74+
]
75+
assert field_names == ["error"]
6976

7077
errors = [f for f in dst_lyr]
7178

@@ -498,23 +505,46 @@ def test_gdalalg_vector_check_geometry_no_geometry_field(alg):
498505
alg.Run()
499506

500507

501-
def test_gdalalg_vector_check_geometry_include_field(alg):
508+
@pytest.mark.parametrize(
509+
"include_field",
510+
(
511+
["EAS_ID", "AREA"],
512+
["AREA", "EAS_ID"],
513+
["AREA", "AREA", "EAS_ID"],
514+
"ALL",
515+
None,
516+
"NONE",
517+
),
518+
)
519+
def test_gdalalg_vector_check_geometry_include_field(alg, include_field):
502520

503521
alg["input"] = "../ogr/data/poly.shp"
504-
alg["include-field"] = ["EAS_ID", "AREA"]
522+
if include_field is not None:
523+
alg["include-field"] = include_field
505524
alg["output-format"] = "stream"
506525
alg["include-valid"] = True
507526

508527
assert alg.Run()
509528
dst_ds = alg["output"].GetDataset()
510529
dst_lyr = dst_ds.GetLayer(0)
511530
dst_defn = dst_lyr.GetLayerDefn()
512-
assert dst_defn.GetFieldDefn(0).GetName() == "EAS_ID"
513-
assert dst_defn.GetFieldDefn(1).GetName() == "AREA"
531+
dst_fields = [
532+
dst_defn.GetFieldDefn(i).GetName() for i in range(dst_defn.GetFieldCount())
533+
]
534+
535+
if include_field in (None, "NONE"):
536+
assert dst_fields == ["error"]
537+
else:
538+
unique_include_fields = list({f: True for f in include_field}.keys())
539+
540+
if include_field == "ALL":
541+
assert dst_fields == ["AREA", "EAS_ID", "PRFEDEA", "error"]
542+
else:
543+
assert dst_fields == unique_include_fields + ["error"]
514544

515-
f = dst_lyr.GetNextFeature()
516-
assert f["EAS_ID"] == 168
517-
assert f["AREA"] == 215229.266
545+
f = dst_lyr.GetNextFeature()
546+
assert f["EAS_ID"] == 168
547+
assert f["AREA"] == 215229.266
518548

519549

520550
def test_gdalalg_vector_check_geometry_include_field_error(alg):
@@ -523,5 +553,5 @@ def test_gdalalg_vector_check_geometry_include_field_error(alg):
523553
alg["include-field"] = "does_not_exist"
524554
alg["output-format"] = "stream"
525555

526-
with pytest.raises(Exception, match="Specified field .* does not exist"):
556+
with pytest.raises(Exception, match="Field .* does not exist"):
527557
alg.Run()

doc/source/programs/gdal_vector_check_geometry.rst

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ Program-Specific Options
5858

5959
.. versionadded:: 3.12.1
6060

61-
Optional field(s) to copy from the input features to the output.
61+
Optional field(s) to copy from the input features to the output. Since GDAL 3.13, the value ``ALL`` can be used to include all fields
62+
from the source layer.
6263

6364
.. option:: --include-valid
6465

gcore/gdalalgorithm.cpp

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6852,6 +6852,52 @@ GDALAlgorithm::GetAutoComplete(std::vector<std::string> &args,
68526852
return ret;
68536853
}
68546854

6855+
/************************************************************************/
6856+
/* GDALAlgorithm::GetFieldIndices() */
6857+
/************************************************************************/
6858+
6859+
bool GDALAlgorithm::GetFieldIndices(const std::vector<std::string> &names,
6860+
OGRLayerH hLayer, std::vector<int> &indices)
6861+
{
6862+
VALIDATE_POINTER1(hLayer, __func__, false);
6863+
6864+
const OGRLayer &layer = *OGRLayer::FromHandle(hLayer);
6865+
6866+
if (names.size() == 1 && names[0] == "ALL")
6867+
{
6868+
const int nSrcFieldCount = layer.GetLayerDefn()->GetFieldCount();
6869+
for (int i = 0; i < nSrcFieldCount; ++i)
6870+
{
6871+
indices.push_back(i);
6872+
}
6873+
}
6874+
else if (!names.empty() && !(names.size() == 1 && names[0] == "NONE"))
6875+
{
6876+
std::set<int> fieldsAdded;
6877+
for (const std::string &osFieldName : names)
6878+
{
6879+
6880+
const int nIdx =
6881+
layer.GetLayerDefn()->GetFieldIndex(osFieldName.c_str());
6882+
6883+
if (nIdx < 0)
6884+
{
6885+
CPLError(CE_Failure, CPLE_AppDefined,
6886+
"Field '%s' does not exist in layer '%s'",
6887+
osFieldName.c_str(), layer.GetName());
6888+
return false;
6889+
}
6890+
6891+
if (fieldsAdded.insert(nIdx).second)
6892+
{
6893+
indices.push_back(nIdx);
6894+
}
6895+
}
6896+
}
6897+
6898+
return true;
6899+
}
6900+
68556901
/************************************************************************/
68566902
/* GDALAlgorithm::ExtractLastOptionAndValue() */
68576903
/************************************************************************/

gcore/gdalalgorithm_cpp.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2995,6 +2995,10 @@ class CPL_DLL GDALAlgorithmRegistry
29952995
std::pair<std::vector<std::pair<GDALAlgorithmArg *, std::string>>, size_t>
29962996
GetArgNamesForCLI() const;
29972997

2998+
/** Get the indices of fields to be included, recognizing the special values "ALL" and "NONE" */
2999+
static bool GetFieldIndices(const std::vector<std::string> &osFieldNames,
3000+
OGRLayerH hLayer, std::vector<int> &anIndices);
3001+
29983002
//! @cond Doxygen_Suppress
29993003
std::string GetUsageForCLIEnd() const;
30003004
//! @endcond

0 commit comments

Comments
 (0)