From 743964393450178289e6717d67cdba04c6206e23 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Fri, 6 Nov 2015 20:08:30 +1100 Subject: [PATCH] Fix delimited text layers set to unknown geometry type if first row has null geometry (fix #13749) --- .../qgsdelimitedtextprovider.cpp | 11 ++- .../python/test_qgsdelimitedtextprovider.py | 18 +++- .../test_qgsdelimitedtextprovider_wanted.py | 83 +++++++++++++++++++ tests/testdata/delimitedtext/test13749.csv | 5 ++ 4 files changed, 112 insertions(+), 5 deletions(-) create mode 100644 tests/testdata/delimitedtext/test13749.csv diff --git a/src/providers/delimitedtext/qgsdelimitedtextprovider.cpp b/src/providers/delimitedtext/qgsdelimitedtextprovider.cpp index a5fcfcecce2..f79810dd714 100644 --- a/src/providers/delimitedtext/qgsdelimitedtextprovider.cpp +++ b/src/providers/delimitedtext/qgsdelimitedtextprovider.cpp @@ -409,6 +409,7 @@ void QgsDelimitedTextProvider::scanFile( bool buildIndexes ) QList couldBeInt; QList couldBeLongLong; QList couldBeDouble; + bool foundFirstGeometry = false; while ( true ) { @@ -458,11 +459,12 @@ void QgsDelimitedTextProvider::scanFile( bool buildIndexes ) if ( mGeometryType == QGis::UnknownGeometry || geom->type() == mGeometryType ) { mGeometryType = geom->type(); - if ( mNumberFeatures == 0 ) + if ( !foundFirstGeometry ) { mNumberFeatures++; mWkbType = type; mExtent = geom->boundingBox(); + foundFirstGeometry = true; } else { @@ -517,7 +519,7 @@ void QgsDelimitedTextProvider::scanFile( bool buildIndexes ) if ( ok ) { - if ( mNumberFeatures > 0 ) + if ( foundFirstGeometry ) { mExtent.combineExtentWith( pt.x(), pt.y() ); } @@ -527,6 +529,7 @@ void QgsDelimitedTextProvider::scanFile( bool buildIndexes ) mExtent.set( pt.x(), pt.y(), pt.x(), pt.y() ); mWkbType = QGis::WKBPoint; mGeometryType = QGis::Point; + foundFirstGeometry = true; } mNumberFeatures++; if ( buildSpatialIndex && qIsFinite( pt.x() ) && qIsFinite( pt.y() ) ) @@ -778,13 +781,15 @@ void QgsDelimitedTextProvider::rescanFile() mNumberFeatures = 0; mExtent = QgsRectangle(); QgsFeature f; + bool foundFirstGeometry = false; while ( fi.nextFeature( f ) ) { if ( mGeometryType != QGis::NoGeometry ) { - if ( mNumberFeatures == 0 ) + if ( !foundFirstGeometry ) { mExtent = f.constGeometry()->boundingBox(); + foundFirstGeometry = true; } else { diff --git a/tests/src/python/test_qgsdelimitedtextprovider.py b/tests/src/python/test_qgsdelimitedtextprovider.py index c404ac2c756..542863b2e4e 100644 --- a/tests/src/python/test_qgsdelimitedtextprovider.py +++ b/tests/src/python/test_qgsdelimitedtextprovider.py @@ -43,7 +43,8 @@ from qgis.core import (QgsProviderRegistry, QgsVectorLayer, QgsFeatureRequest, QgsRectangle, - QgsMessageLog + QgsMessageLog, + QGis ) from utilities import (getQgisTestApp, @@ -190,7 +191,7 @@ def delimitedTextData(testname, filename, requests, verbose, **params): msg = re.sub(r'file\s+.*' + re.escape(filename), 'file ' + filelogname, msg) msg = msg.replace(filepath, filelogname) log.append(msg) - return dict(fields=fields, fieldTypes=fieldTypes, data=data, log=log, uri=uri) + return dict(fields=fields, fieldTypes=fieldTypes, data=data, log=log, uri=uri, geometryType=layer.geometryType()) def printWanted(testname, result): @@ -206,6 +207,7 @@ def printWanted(testname, result): print prefix + "wanted={}" print prefix + "wanted['uri']=" + repr(result['uri']) print prefix + "wanted['fieldTypes']=" + repr(result['fieldTypes']) + print prefix + "wanted['geometryType']=" + repr(result['geometryType']) print prefix + "wanted['data']={" for k in sorted(data.keys()): row = data[k] @@ -272,6 +274,10 @@ def runTest(file, requests, **params): msg = "Layer field types ({0}) doesn't match expected ({1})".format( result['fieldTypes'], wanted['fieldTypes']) failures.append(msg) + if result['geometryType'] != wanted['geometryType']: + msg = "Layer geometry type ({0}) doesn't match expected ({1})".format( + result['geometryType'], wanted['geometryType']) + failures.append(msg) wanted_data = wanted['data'] for id in sorted(wanted_data.keys()): wrec = wanted_data[id] @@ -728,5 +734,13 @@ class TestQgsDelimitedTextProviderOther(TestCase): requests = None runTest(filename, requests, **params) + def test_039_issue_13749(self): + # First record contains missing geometry + filename = 'test13749.csv' + params = {'yField': 'geom_y', 'xField': 'geom_x', 'type': 'csv'} + requests = None + runTest(filename, requests, **params) + + if __name__ == '__main__': unittest.main() diff --git a/tests/src/python/test_qgsdelimitedtextprovider_wanted.py b/tests/src/python/test_qgsdelimitedtextprovider_wanted.py index 4de835b785b..d10b782e4db 100644 --- a/tests/src/python/test_qgsdelimitedtextprovider_wanted.py +++ b/tests/src/python/test_qgsdelimitedtextprovider_wanted.py @@ -3,6 +3,7 @@ def test_002_load_csv_file(): wanted = {} wanted['uri'] = u'file://test.csv?geomType=none&type=csv' wanted['fieldTypes'] = ['integer', 'text', 'text', 'text', 'text'] + wanted['geometryType'] = 4 wanted['data'] = { 2: { 'id': u'1', @@ -67,6 +68,7 @@ def test_003_field_naming(): wanted = {} wanted['uri'] = u'file://testfields.csv?geomType=none&type=csv' wanted['fieldTypes'] = ['integer', 'text', 'text', 'text', 'text', 'text', 'text', 'text', 'text', 'text', 'text', 'text'] + wanted['geometryType'] = 4 wanted['data'] = { 2: { 'id': u'1', @@ -93,6 +95,7 @@ def test_004_max_fields(): wanted = {} wanted['uri'] = u'file://testfields.csv?geomType=none&maxFields=7&type=csv' wanted['fieldTypes'] = ['integer', 'text', 'text', 'text', 'text', 'text', 'text'] + wanted['geometryType'] = 4 wanted['data'] = { 2: { 'id': u'1', @@ -114,6 +117,7 @@ def test_005_load_whitespace(): wanted = {} wanted['uri'] = u'file://test.space?geomType=none&type=whitespace' wanted['fieldTypes'] = ['integer', 'text', 'text', 'text', 'text', 'text'] + wanted['geometryType'] = 4 wanted['data'] = { 2: { 'id': u'1', @@ -184,6 +188,7 @@ def test_006_quote_escape(): wanted = {} wanted['uri'] = u'file://test.pipe?geomType=none"e="&delimiter=|&escape=\\' wanted['fieldTypes'] = ['integer', 'text', 'text', 'text', 'text', 'text'] + wanted['geometryType'] = 4 wanted['data'] = { 2: { 'id': u'1', @@ -274,6 +279,7 @@ def test_007_multiple_quote(): wanted = {} wanted['uri'] = u'file://test.quote?geomType=none"e=\'"&type=csv&escape="\'' wanted['fieldTypes'] = ['integer', 'text', 'text', 'text'] + wanted['geometryType'] = 4 wanted['data'] = { 2: { 'id': u'1', @@ -339,6 +345,7 @@ def test_008_badly_formed_quotes(): wanted = {} wanted['uri'] = u'file://test.badquote?geomType=none"e="&type=csv&escape="' wanted['fieldTypes'] = ['integer', 'text', 'text', 'text'] + wanted['geometryType'] = 4 wanted['data'] = { 4: { 'id': u'3', @@ -363,6 +370,7 @@ def test_009_skip_lines(): wanted = {} wanted['uri'] = u'file://test2.csv?geomType=none&skipLines=2&type=csv&useHeader=no' wanted['fieldTypes'] = ['integer', 'text', 'text'] + wanted['geometryType'] = 4 wanted['data'] = { 3: { 'id': u'3', @@ -382,6 +390,7 @@ def test_010_read_coordinates(): wanted = {} wanted['uri'] = u'file://testpt.csv?yField=geom_y&xField=geom_x&type=csv' wanted['fieldTypes'] = ['integer', 'text', 'double', 'double'] + wanted['geometryType'] = 0 wanted['data'] = { 2: { 'id': u'1', @@ -421,6 +430,7 @@ def test_011_read_wkt(): wanted = {} wanted['uri'] = u'file://testwkt.csv?delimiter=|&type=csv&wktField=geom_wkt' wanted['fieldTypes'] = ['integer', 'text'] + wanted['geometryType'] = 0 wanted['data'] = { 2: { 'id': u'1', @@ -467,6 +477,7 @@ def test_012_read_wkt_point(): wanted = {} wanted['uri'] = u'file://testwkt.csv?geomType=point&delimiter=|&type=csv&wktField=geom_wkt' wanted['fieldTypes'] = ['integer', 'text'] + wanted['geometryType'] = 0 wanted['data'] = { 2: { 'id': u'1', @@ -513,6 +524,7 @@ def test_013_read_wkt_line(): wanted = {} wanted['uri'] = u'file://testwkt.csv?geomType=line&delimiter=|&type=csv&wktField=geom_wkt' wanted['fieldTypes'] = ['integer', 'text'] + wanted['geometryType'] = 1 wanted['data'] = { 4: { 'id': u'3', @@ -559,6 +571,7 @@ def test_014_read_wkt_polygon(): wanted = {} wanted['uri'] = u'file://testwkt.csv?geomType=polygon&delimiter=|&type=csv&wktField=geom_wkt' wanted['fieldTypes'] = ['integer', 'text'] + wanted['geometryType'] = 2 wanted['data'] = { 6: { 'id': u'5', @@ -587,6 +600,7 @@ def test_015_read_dms_xy(): wanted = {} wanted['uri'] = u'file://testdms.csv?yField=lat&xField=lon&type=csv&xyDms=yes' wanted['fieldTypes'] = ['integer', 'text', 'text', 'text'] + wanted['geometryType'] = 0 wanted['data'] = { 3: { 'id': u'1', @@ -758,6 +772,7 @@ def test_016_decimal_point(): wanted = {} wanted['uri'] = u'file://testdp.csv?yField=geom_y&xField=geom_x&type=csv&delimiter=;&decimalPoint=,' wanted['fieldTypes'] = ['integer', 'text', 'double', 'double', 'double', 'text'] + wanted['geometryType'] = 0 wanted['data'] = { 2: { 'id': u'1', @@ -788,6 +803,7 @@ def test_017_regular_expression_1(): wanted = {} wanted['uri'] = u'file://testre.txt?geomType=none&trimFields=Y&delimiter=RE(?:GEXP)?&type=regexp' wanted['fieldTypes'] = ['integer', 'text', 'text', 'text'] + wanted['geometryType'] = 4 wanted['data'] = { 2: { 'id': u'1', @@ -814,6 +830,7 @@ def test_018_regular_expression_2(): wanted = {} wanted['uri'] = u'file://testre.txt?geomType=none&trimFields=Y&delimiter=(RE)(GEXP)?&type=regexp' wanted['fieldTypes'] = ['integer', 'text', 'text', 'text', 'text', 'text', 'text', 'text', 'text', 'text'] + wanted['geometryType'] = 4 wanted['data'] = { 2: { 'id': u'1', @@ -852,6 +869,7 @@ def test_019_regular_expression_3(): wanted = {} wanted['uri'] = u'file://testre2.txt?geomType=none&trimFields=Y&delimiter=^(.{5})(.{30})(.{5,})&type=regexp' wanted['fieldTypes'] = ['integer', 'text', 'text'] + wanted['geometryType'] = 4 wanted['data'] = { 2: { 'id': u'1', @@ -881,6 +899,7 @@ def test_020_regular_expression_4(): wanted = {} wanted['uri'] = u'file://testre3.txt?geomType=none&delimiter=x?&type=regexp' wanted['fieldTypes'] = ['text', 'text', 'text', 'text', 'text', 'text', 'text'] + wanted['geometryType'] = 4 wanted['data'] = { 2: { 'id': u'f', @@ -904,6 +923,7 @@ def test_021_regular_expression_5(): wanted = {} wanted['uri'] = u'file://testre3.txt?geomType=none&delimiter=\\b&type=regexp' wanted['fieldTypes'] = ['text', 'text', 'text'] + wanted['geometryType'] = 4 wanted['data'] = { 2: { 'id': u'fi', @@ -923,6 +943,7 @@ def test_022_utf8_encoded_file(): wanted = {} wanted['uri'] = u'file://testutf8.csv?geomType=none&delimiter=|&type=csv&encoding=utf-8' wanted['fieldTypes'] = ['integer', 'text', 'text'] + wanted['geometryType'] = 4 wanted['data'] = { 2: { 'id': u'1', @@ -940,6 +961,7 @@ def test_023_latin1_encoded_file(): wanted = {} wanted['uri'] = u'file://testlatin1.csv?geomType=none&delimiter=|&type=csv&encoding=latin1' wanted['fieldTypes'] = ['integer', 'text', 'text'] + wanted['geometryType'] = 4 wanted['data'] = { 2: { 'id': u'1', @@ -957,6 +979,7 @@ def test_024_filter_rect_xy(): wanted = {} wanted['uri'] = u'file://testextpt.txt?yField=y&delimiter=|&type=csv&xField=x' wanted['fieldTypes'] = ['integer', 'text', 'integer', 'integer'] + wanted['geometryType'] = 0 wanted['data'] = { 2: { 'id': u'1', @@ -1001,6 +1024,7 @@ def test_025_filter_rect_wkt(): wanted = {} wanted['uri'] = u'file://testextw.txt?delimiter=|&type=csv&wktField=wkt' wanted['fieldTypes'] = ['integer', 'text'] + wanted['geometryType'] = 1 wanted['data'] = { 2: { 'id': u'1', @@ -1061,6 +1085,7 @@ def test_026_filter_fid(): wanted = {} wanted['uri'] = u'file://test.csv?geomType=none&type=csv' wanted['fieldTypes'] = ['integer', 'text', 'text', 'text', 'text'] + wanted['geometryType'] = 4 wanted['data'] = { 3: { 'id': u'2', @@ -1100,6 +1125,7 @@ def test_027_filter_attributes(): wanted = {} wanted['uri'] = u'file://test.csv?geomType=none&type=csv' wanted['fieldTypes'] = ['integer', 'text', 'text', 'text', 'text'] + wanted['geometryType'] = 4 wanted['data'] = { 2: { 'id': u'None', @@ -1209,6 +1235,7 @@ def test_028_substring_test(): wanted = {} wanted['uri'] = u'file://test.csv?geomType=none&type=csv&subset=id%20%25%202%20%3D%201' wanted['fieldTypes'] = ['integer', 'text', 'text', 'text', 'text'] + wanted['geometryType'] = 4 wanted['data'] = { 2: { 'id': u'1', @@ -1246,6 +1273,7 @@ def test_029_file_watcher(): wanted = {} wanted['uri'] = u'file://file?geomType=none&type=csv&watchFile=yes' wanted['fieldTypes'] = ['integer', 'text'] + wanted['geometryType'] = 4 wanted['data'] = { 3: { 'id': u'2', @@ -1359,6 +1387,7 @@ def test_030_filter_rect_xy_spatial_index(): wanted = {} wanted['uri'] = u'file://testextpt.txt?spatialIndex=Y&yField=y&delimiter=|&type=csv&xField=x' wanted['fieldTypes'] = ['integer', 'text', 'integer', 'integer'] + wanted['geometryType'] = 0 wanted['data'] = { 2: { 'id': u'1', @@ -1547,6 +1576,7 @@ def test_031_filter_rect_wkt_spatial_index(): wanted = {} wanted['uri'] = u'file://testextw.txt?spatialIndex=Y&delimiter=|&type=csv&wktField=wkt' wanted['fieldTypes'] = ['integer', 'text'] + wanted['geometryType'] = 1 wanted['data'] = { 2: { 'id': u'1', @@ -1679,6 +1709,7 @@ def test_032_filter_rect_wkt_create_spatial_index(): wanted = {} wanted['uri'] = u'file://testextw.txt?delimiter=|&type=csv&wktField=wkt' wanted['fieldTypes'] = ['integer', 'text'] + wanted['geometryType'] = 1 wanted['data'] = { 2: { 'id': u'1', @@ -1877,6 +1908,7 @@ def test_033_reset_subset_string(): wanted = {} wanted['uri'] = u'file://test.csv?geomType=none&type=csv' wanted['fieldTypes'] = ['integer', 'text', 'text', 'text', 'text'] + wanted['geometryType'] = 4 wanted['data'] = { 2: { 'id': u'1', @@ -2040,6 +2072,7 @@ def test_034_csvt_file(): wanted = {} wanted['uri'] = u'file://testcsvt.csv?geomType=none&type=csv' wanted['fieldTypes'] = ['integer', 'text', 'integer', 'double', 'text', 'text', 'text', 'text', 'text', 'text', 'longlong', 'longlong'] + wanted['geometryType'] = 4 wanted['data'] = { 2: { 'id': u'1', @@ -2082,6 +2115,7 @@ def test_035_csvt_file2(): wanted = {} wanted['uri'] = u'file://testcsvt2.txt?geomType=none&delimiter=|&type=csv' wanted['fieldTypes'] = ['integer', 'text', 'integer', 'double', 'integer', 'text', 'integer'] + wanted['geometryType'] = 4 wanted['data'] = { 2: { 'id': u'1', @@ -2114,6 +2148,7 @@ def test_036_csvt_file_invalid_types(): wanted = {} wanted['uri'] = u'file://testcsvt3.csv?geomType=none&type=csv' wanted['fieldTypes'] = ['integer', 'text', 'integer', 'double', 'integer', 'text', 'text'] + wanted['geometryType'] = 4 wanted['data'] = { 2: { 'id': u'1', @@ -2149,6 +2184,7 @@ def test_037_csvt_file_invalid_file(): wanted = {} wanted['uri'] = u'file://testcsvt4.csv?geomType=none&type=csv' wanted['fieldTypes'] = ['integer', 'text', 'integer', 'double', 'integer', 'text', 'text'] + wanted['geometryType'] = 4 wanted['data'] = { 2: { 'id': u'1', @@ -2181,6 +2217,7 @@ def test_038_type_inference(): wanted = {} wanted['uri'] = u'file://testtypes.csv?yField=lat&xField=lon&type=csv' wanted['fieldTypes'] = ['text', 'double', 'double', 'text', 'text', 'integer', 'longlong', 'double', 'text'] + wanted['geometryType'] = 0 wanted['data'] = { 2: { 'id': u'line1', @@ -2269,3 +2306,49 @@ def test_038_type_inference(): } wanted['log'] = [] return wanted + + +def test_039_issue_13749(): + wanted = {} + wanted['uri'] = u'file://test13749.csv?yField=geom_y&xField=geom_x&type=csv' + wanted['fieldTypes'] = ['integer', 'text', 'double', 'double'] + wanted['geometryType'] = 0 + wanted['data'] = { + 2: { + 'id': u'1', + 'description': u'No geom', + 'geom_x': u'NULL', + 'geom_y': u'NULL', + '#fid': 2, + '#geometry': 'None', + }, + 3: { + 'id': u'2', + 'description': u'Point1', + 'geom_x': u'11.0', + 'geom_y': u'22.0', + '#fid': 3, + '#geometry': 'Point (11 22)', + }, + 4: { + 'id': u'3', + 'description': u'Point2', + 'geom_x': u'15.0', + 'geom_y': u'23.0', + '#fid': 4, + '#geometry': 'Point (15 23)', + }, + 5: { + 'id': u'4', + 'description': u'Point3', + 'geom_x': u'13.0', + 'geom_y': u'23.0', + '#fid': 5, + '#geometry': 'Point (13 23)', + }, + } + wanted['log'] = [ + u'Errors in file test13749.csv', + u'1 records have missing geometry definitions', + ] + return wanted diff --git a/tests/testdata/delimitedtext/test13749.csv b/tests/testdata/delimitedtext/test13749.csv new file mode 100644 index 00000000000..9ac56c7d3ac --- /dev/null +++ b/tests/testdata/delimitedtext/test13749.csv @@ -0,0 +1,5 @@ +id,description,geom_x,geom_y +1,No geom,, +2,Point1,11,22 +3,Point2,15,23 +4,Point3,13.0,23.0