From af1dee9f7bf24687f0b0ab01b31a688157726e47 Mon Sep 17 00:00:00 2001 From: Karim Bahgat Date: Mon, 11 Apr 2022 11:21:30 +0200 Subject: [PATCH] Fix errors when writing to individual file-handles, fix corresponding tests Previous tests failed to detect the errors because the errors only happen when trying to add a record or shape; fixed. Files were not closed and thus did not flush the updated header data; fixed. Made sure shx records were only written if shx file was available. --- shapefile.py | 18 ++-- shapefiles/test/balancing.dbf | Bin 804 -> 804 bytes shapefiles/test/contextwriter.dbf | Bin 65 -> 65 bytes shapefiles/test/corrupt_too_long.dbf | Bin 580 -> 580 bytes shapefiles/test/dtype.dbf | Bin 259 -> 259 bytes shapefiles/test/edit.dbf | Bin 11368 -> 11368 bytes shapefiles/test/line.dbf | Bin 116 -> 116 bytes shapefiles/test/linem.dbf | Bin 116 -> 116 bytes shapefiles/test/linez.dbf | Bin 116 -> 116 bytes shapefiles/test/merge.dbf | Bin 2355059 -> 2355059 bytes shapefiles/test/multipatch.dbf | Bin 116 -> 116 bytes shapefiles/test/multipoint.dbf | Bin 116 -> 116 bytes shapefiles/test/onlydbf.dbf | Bin 65 -> 65 bytes shapefiles/test/point.dbf | Bin 116 -> 116 bytes shapefiles/test/polygon.dbf | Bin 116 -> 116 bytes shapefiles/test/shapetype.dbf | Bin 65 -> 65 bytes shapefiles/test/testfile.dbf | Bin 65 -> 65 bytes test_shapefile.py | 131 +++++++++++++++++++++------ 18 files changed, 111 insertions(+), 38 deletions(-) diff --git a/shapefile.py b/shapefile.py index 0c665e0..797338e 100644 --- a/shapefile.py +++ b/shapefile.py @@ -1861,14 +1861,13 @@ def close(self): if self.dbf and dbf_open: self.__dbfHeader() - # Close files, if target is a filepath - if self.target: - for attribute in (self.shp, self.shx, self.dbf): - if hasattr(attribute, 'close'): - try: - attribute.close() - except IOError: - pass + # Close files + for attribute in (self.shp, self.shx, self.dbf): + if hasattr(attribute, 'close'): + try: + attribute.close() + except IOError: + pass def __getFileObj(self, f): """Safety handler to verify file-like objects""" @@ -2088,7 +2087,8 @@ def shape(self, s): "not: %r" % s) # Write to file offset,length = self.__shpRecord(s) - self.__shxRecord(offset, length) + if self.shx: + self.__shxRecord(offset, length) def __shpRecord(self, s): f = self.__getFileObj(self.shp) diff --git a/shapefiles/test/balancing.dbf b/shapefiles/test/balancing.dbf index 0da4a8da6ee1ad0e3dd7ddd10527e0d17d47f771..62476f8afdb27011511c0c122627abb3ff3347a6 100644 GIT binary patch delta 13 UcmZ3&wuFs^xr&8*BZ~qv02bu}y#N3J delta 13 UcmZ3&wuFs^xr$M0BZ~qv02dwt$p8QV diff --git a/shapefiles/test/contextwriter.dbf b/shapefiles/test/contextwriter.dbf index 58e825db94ecd349e3594e1f2e55aa79909201b2..ae080bfe0f7efd831a8aa4f66986c8c9caa203b7 100644 GIT binary patch delta 10 RcmZ>CWMQsi;hxB13jhix0p$Py delta 10 RcmZ>CWMQsil$yw53jhjJ0r3C; diff --git a/shapefiles/test/corrupt_too_long.dbf b/shapefiles/test/corrupt_too_long.dbf index b92d4384e7ba1d4b0e5dfdd5337e890458e6b5ed..0acc46a6282883259fe791ec188e752dd4ef0f1a 100644 GIT binary patch delta 13 UcmX@Ya)gD2xr&8*BZ~tQ02%lL7ytkO delta 13 UcmX@Ya)gD2xr&8rBZ~tQ02%WG7XSbN diff --git a/shapefiles/test/dtype.dbf b/shapefiles/test/dtype.dbf index 3dafac7e6861627ecd90aaac9eadc88e644e0c4a..ad798c4e7003af4c2f95e26daae7c64f8c005f41 100644 GIT binary patch delta 12 TcmZo>YGz_#u43Vy$nqZm5rG3Q delta 12 TcmZo>YGz_#u40s$$nqZm5v~I} diff --git a/shapefiles/test/edit.dbf b/shapefiles/test/edit.dbf index 7f6d890315595a959e57473cc3b75bb1bef5d717..bc956931cc04e5ef9fe848c71d616d49709f3090 100644 GIT binary patch delta 13 UcmaD6@gjnSxr&8*BTI@703`whRsaA1 delta 13 UcmaD6@gjnSxr$M0BTI@703|yFVgLXD diff --git a/shapefiles/test/line.dbf b/shapefiles/test/line.dbf index 8ab0a1ffe79f6c7bc0b06cfcfefd0983df64011c..21802a0673341558bb465662e7c66f8d37764a2a 100644 GIT binary patch delta 10 RcmXRZVPURf;hxA+000gm0)7Ah delta 10 RcmXRZVPURfl$yv=000h80*U|t diff --git a/shapefiles/test/linem.dbf b/shapefiles/test/linem.dbf index c34c495cb990407dcbcadb0e9ab000c18fa3c6fa..aec663ca19f9a3eb8357c5097bb2b8310d00e8bc 100644 GIT binary patch delta 10 RcmXRZVPURf;hxA+000gm0)7Ah delta 10 RcmXRZVPURfl$yv=000h80*U|t diff --git a/shapefiles/test/linez.dbf b/shapefiles/test/linez.dbf index df8b7f42cbcbbad0ae87712066d435a04dd3c3e7..175ad0e4d33833fd1f0fdf8b43b897797412101c 100644 GIT binary patch delta 10 RcmXRZVPURf;hxA+000gm0)7Ah delta 10 RcmXRZVPURfl$yv=000h80*U|t diff --git a/shapefiles/test/merge.dbf b/shapefiles/test/merge.dbf index 2d01d9d7524ef23d7405808ebe8f7491c6a48b9a..a620dfdeb34428d32780879a5340fff38636e253 100644 GIT binary patch delta 122 zcmWN@OBO-^06@`$7ZpWCrRcjr^JWd+xYgK>Eg0B92KSsh4ZmQT5q(I%NFbp^GJhel tB(jv1tR)w^YPrZ&8foPwcX`NDUOn$_`~xM59G3t9 delta 122 zcmWN@xe!7PiY6b{`UteuNCWMQsi;hxB13jhix0p$Py delta 10 RcmZ>CWMQsil$yw53jhjJ0r3C; diff --git a/shapefiles/test/point.dbf b/shapefiles/test/point.dbf index db78fec66187ca68eb872d60389e11198f3e150a..646e45bb63bd927df8c29672005af685a61c5d74 100644 GIT binary patch delta 10 RcmXRZVPURf;hxA+000gm0)7Ah delta 10 RcmXRZVPURfl$yv=000h80*U|t diff --git a/shapefiles/test/polygon.dbf b/shapefiles/test/polygon.dbf index b64620c9527e78576d40aa8efd01e28e3101c803..bbad177603b7682b46e5b58602a16f429f13bce3 100644 GIT binary patch delta 10 RcmXRZVPURf;hxA+000gm0)7Ah delta 10 RcmXRZVPURfl$yv=000h80*U|t diff --git a/shapefiles/test/shapetype.dbf b/shapefiles/test/shapetype.dbf index 58e825db94ecd349e3594e1f2e55aa79909201b2..ae080bfe0f7efd831a8aa4f66986c8c9caa203b7 100644 GIT binary patch delta 10 RcmZ>CWMQsi;hxB13jhix0p$Py delta 10 RcmZ>CWMQsil$yw53jhjJ0r3C; diff --git a/shapefiles/test/testfile.dbf b/shapefiles/test/testfile.dbf index 58e825db94ecd349e3594e1f2e55aa79909201b2..ae080bfe0f7efd831a8aa4f66986c8c9caa203b7 100644 GIT binary patch delta 10 RcmZ>CWMQsi;hxB13jhix0p$Py delta 10 RcmZ>CWMQsil$yw53jhjJ0r3C; diff --git a/test_shapefile.py b/test_shapefile.py index 9bf5e29..95cd5a6 100644 --- a/test_shapefile.py +++ b/test_shapefile.py @@ -1019,38 +1019,93 @@ def test_write_shp_only(tmpdir): shp argument to the shapefile writer creates just a shp file. """ - filename = tmpdir.join("test.shp").strpath - with shapefile.Writer(shp=filename) as writer: - pass + filename = tmpdir.join("test").strpath + with shapefile.Writer(shp=open(filename+'.shp','wb')) as writer: + writer.point(1, 1) + assert writer.shp and not writer.shx and not writer.dbf + assert writer.shpNum == 1 + assert len(writer) == 1 + assert writer.shp.closed == True # assert test.shp exists - assert os.path.exists(filename) + assert os.path.exists(filename+'.shp') + + # test that can read shapes + with shapefile.Reader(shp=open(filename+'.shp','rb')) as reader: + assert reader.shp and not reader.shx and not reader.dbf + assert (reader.numRecords, reader.numShapes) == (None, None) # numShapes is unknown in the absence of shx file + assert len(reader.shapes()) == 1 # assert test.shx does not exist - assert not os.path.exists(tmpdir.join("test.shx").strpath) + assert not os.path.exists(filename+'.shx') # assert test.dbf does not exist - assert not os.path.exists(tmpdir.join("test.dbf").strpath) + assert not os.path.exists(filename+'.dbf') -def test_write_shx_only(tmpdir): +def test_write_shp_shx_only(tmpdir): """ - Assert that specifying just the + Assert that specifying just the shp and shx argument to the shapefile writer - creates just a shx file. + creates just a shp and shx file. """ - filename = tmpdir.join("test.shx").strpath - with shapefile.Writer(shx=filename) as writer: - pass + filename = tmpdir.join("test").strpath + with shapefile.Writer(shp=open(filename+'.shp','wb'), shx=open(filename+'.shx','wb')) as writer: + writer.point(1, 1) + assert writer.shp and writer.shx and not writer.dbf + assert writer.shpNum == 1 + assert len(writer) == 1 + assert writer.shp.closed == writer.shx.closed == True + + # assert test.shp exists + assert os.path.exists(filename+'.shp') # assert test.shx exists - assert os.path.exists(filename) + assert os.path.exists(filename+'.shx') - # assert test.shp does not exist - assert not os.path.exists(tmpdir.join("test.shp").strpath) + # test that can read shapes and offsets + with shapefile.Reader(shp=open(filename+'.shp','rb'), shx=open(filename+'.shx','rb')) as reader: + assert reader.shp and reader.shx and not reader.dbf + assert (reader.numRecords, reader.numShapes) == (None, 1) + reader.shape(0) # trigger reading of shx offsets + assert len(reader._offsets) == 1 + assert len(reader.shapes()) == 1 # assert test.dbf does not exist - assert not os.path.exists(tmpdir.join("test.dbf").strpath) + assert not os.path.exists(filename+'.dbf') + + +def test_write_shp_dbf_only(tmpdir): + """ + Assert that specifying just the + shp and dbf argument to the shapefile writer + creates just a shp and dbf file. + """ + filename = tmpdir.join("test").strpath + with shapefile.Writer(shp=open(filename+'.shp','wb'), dbf=open(filename+'.dbf','wb')) as writer: + writer.field('field1', 'C') # required to create a valid dbf file + writer.record('value') + writer.point(1, 1) + assert writer.shp and not writer.shx and writer.dbf + assert writer.shpNum == writer.recNum == 1 + assert len(writer) == 1 + assert writer.shp.closed == writer.dbf.closed == True + + # assert test.shp exists + assert os.path.exists(filename+'.shp') + + # assert test.dbf exists + assert os.path.exists(filename+'.dbf') + + # test that can read records and shapes + with shapefile.Reader(shp=open(filename+'.shp','rb'), dbf=open(filename+'.dbf','rb')) as reader: + assert reader.shp and not reader.shx and reader.dbf + assert (reader.numRecords, reader.numShapes) == (1, None) # numShapes is unknown in the absence of shx file + assert len(reader.records()) == 1 + assert len(reader.shapes()) == 1 + + # assert test.shx does not exist + assert not os.path.exists(filename+'.shx') def test_write_dbf_only(tmpdir): @@ -1059,18 +1114,29 @@ def test_write_dbf_only(tmpdir): dbf argument to the shapefile writer creates just a dbf file. """ - filename = tmpdir.join("test.dbf").strpath - with shapefile.Writer(dbf=filename) as writer: + filename = tmpdir.join("test").strpath + with shapefile.Writer(dbf=open(filename+'.dbf','wb')) as writer: writer.field('field1', 'C') # required to create a valid dbf file + writer.record('value') + assert not writer.shp and not writer.shx and writer.dbf + assert writer.recNum == 1 + assert len(writer) == 1 + assert writer.dbf.closed == True # assert test.dbf exists - assert os.path.exists(filename) + assert os.path.exists(filename+'.dbf') + + # test that can read records + with shapefile.Reader(dbf=open(filename+'.dbf','rb')) as reader: + assert not writer.shp and not writer.shx and writer.dbf + assert (reader.numRecords, reader.numShapes) == (1, None) + assert len(reader.records()) == 1 # assert test.shp does not exist - assert not os.path.exists(tmpdir.join("test.shp").strpath) + assert not os.path.exists(filename+'.shp') # assert test.shx does not exist - assert not os.path.exists(tmpdir.join("test.shx").strpath) + assert not os.path.exists(filename+'.shx') def test_write_default_shp_shx_dbf(tmpdir): @@ -1133,10 +1199,10 @@ def test_write_record(tmpdir): with shapefile.Writer(filename) as writer: writer.autoBalance = True - writer.field('one', 'C') # many under length limit - writer.field('two', 'C') # 1 under length limit - writer.field('three', 'C') # at length limit - writer.field('four', 'C') # 1 over length limit + writer.field('one', 'C') + writer.field('two', 'C') + writer.field('three', 'C') + writer.field('four', 'C') values = ['one','two','three','four'] writer.record(*values) @@ -1160,10 +1226,10 @@ def test_write_partial_record(tmpdir): with shapefile.Writer(filename) as writer: writer.autoBalance = True - writer.field('one', 'C') # many under length limit - writer.field('two', 'C') # 1 under length limit - writer.field('three', 'C') # at length limit - writer.field('four', 'C') # 1 over length limit + writer.field('one', 'C') + writer.field('two', 'C') + writer.field('three', 'C') + writer.field('four', 'C') values = ['one','two'] writer.record(*values) @@ -1220,4 +1286,11 @@ def test_write_empty_shapefile(tmpdir, shape_type): w.field('field1', 'C') # required to create a valid dbf file with shapefile.Reader(filename) as r: + # test correct shape type assert r.shapeType == shape_type + # test length 0 + assert len(r) == r.numRecords == r.numShapes == 0 + # test records are empty + assert len(r.records()) == 0 + # test shapes are empty + assert len(r.shapes()) == 0