write to shapefile #15

ErickChacon · 2023-05-03T17:22:53Z

PR to be able to export to shapefile using Shapefile.jl and improve geointerface functionality. Whem geometries are Multi, exporting to .shp is quite easy with SHP.write(fname, geotable), otherwise we need to convert to Multi and then write.

src/GeoTables.jl

src/conversion.jl

src/GeoTables.jl

ErickChacon · 2023-05-03T18:37:36Z

@juliohm I started a discussion in Shapefile.jl, let's see what they say. I will also like to be able to write to other formats using ArchGDAL. Should I start that in a separate PR? I think we can use something similar to GeoDataFrames.jl:

https://github.com/evetion/GeoDataFrames.jl/blob/fec887c71124adf1dd11c33878ed6186aba10bd3/src/io.jl#L73C1-L164

Let me know your thoughts.

juliohm · 2023-05-03T18:41:58Z

Thank you @ErickChacon for these great additions. Can you explain why our save doesn't cover the ArchGDAL.jl case? If we pass a geotable with geometry column following the GeoInterface.jl shouldn't ArchGDAL.jl provide a similar AG.write function?

README.md

Co-authored-by: Júlio Hoffimann <julio.hoffimann@gmail.com>

ErickChacon · 2023-05-03T20:20:34Z

Thank you @ErickChacon for these great additions. Can you explain why our save doesn't cover the ArchGDAL.jl case? If we pass a geotable with geometry column following the GeoInterface.jl shouldn't ArchGDAL.jl provide a similar AG.write function?

Last time I checked it, there was no functionality to write from a table. I needed to manually convert a table to an ArchGDAL dataset iterating over rows to push features. I will check if this has been improved.

juliohm · 2023-05-03T20:22:22Z

That would be great. Maybe it is another situation where we need to fix the issue upstream in ArchGDAL.jl, adding a write function there before coming back here. This is a good exercise to improve the status of these loaders.

ErickChacon · 2023-05-03T23:39:49Z

That would be great. Maybe it is another situation where we need to fix the issue upstream in ArchGDAL.jl, adding a write function there before coming back here. This is a good exercise to improve the status of these loaders.

Just to let you know that there is an attempt at yeesian/ArchGDAL.jl#243 to create an IFeatureLayer from a Table. Once something like that is merged, we could easily use ArchGDAL to export to other format types.

With respect to Shapefile.jl, I started an issue, should we accept our current fix until that issue is solved? Or should we keep the adequate code (removing the conditional lines) and accepting that an error will be thrown when attempting to write a Chain and PolyArea to a .shp file?

juliohm · 2023-05-03T23:42:51Z

Let's remove the conditional and wait for the Shapefile.jl fix then. I can merge it here soon and release a new patch. Em qua., 3 de mai. de 2023 20:40, Dr. Erick A. Chacón Montalván < ***@***.***> escreveu:

…

That would be great. Maybe it is another situation where we need to fix the issue upstream in ArchGDAL.jl, adding a write function there before coming back here. This is a good exercise to improve the status of these loaders. Just to let you know that there is an attempt at yeesian/ArchGDAL.jl#243 <yeesian/ArchGDAL.jl#243> to create an IFeatureLayer from a Table. Once something like that is merged, we could easily use ArchGDAL to export to other format types. With respect to Shapefile.jl, I started an issue, should we accept our current fix until that issue is solved? Or should we keep the adequate code (removing the conditional lines) and accepting that an error will be thrown when attempting to write a Chain and PolyArea to a .shp file? — Reply to this email directly, view it on GitHub <#15 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZQW3MB7LKGBFA6T2PJLHTXELUE7ANCNFSM6AAAAAAXUWU53Q> . You are receiving this because you were mentioned.Message ID: ***@***.***>

juliohm · 2023-05-03T23:55:44Z

Do we need more tests for the save functionality?

juliohm · 2023-05-03T23:58:11Z

I am wondering why our tests are failing only on GitHub Actions. Any clue why the HTTP requests are failing from GADM.jl?

ErickChacon · 2023-05-04T00:13:26Z

Do we need more tests for the save functionality?

Yes, I forgot about it. Should I do it similar to the existing test with "geojson"?

ErickChacon · 2023-05-04T00:14:28Z

I am wondering why our tests are failing only on GitHub Actions. Any clue why the HTTP requests are failing from GADM.jl?

No, I haven't checked it. I will check it.

juliohm · 2023-05-04T00:14:43Z

Should I do it similar to the existing test with "geojson"?

That would be great, but feel free to add other tests you have in mind too.

juliohm · 2023-05-04T00:19:47Z

I've updated the test/Project.toml in GADM.jl to see if the issue was coming from there, and learned that ArchGDAL.jl is not even building there in GitHub Actions: JuliaGeo/GADM.jl#49

I really dislike the whole GDAL experience, so full of issues...

ErickChacon · 2023-05-04T00:21:33Z

Should I do it similar to the existing test with "geojson"?

That would be great, but feel free to add other tests you have in mind too.

Ok. Should it be better to make the writing test inside the formats testset, so we can take advantage of the already loaded data?

juliohm · 2023-05-04T00:24:07Z

Makes sense to move the writing tests to inside the other test sets. Maybe we can remove the outer "load" test set then?

ErickChacon · 2023-05-04T00:30:01Z

Makes sense to move the writing tests to inside the other test sets. Maybe we can remove the outer "load" test set then?

Yes, that is what I have in mind. I will do that.

ErickChacon · 2023-05-04T00:40:21Z

I've updated the test/Project.toml in GADM.jl to see if the issue was coming from there, and learned that ArchGDAL.jl is not even building there in GitHub Actions: JuliaGeo/GADM.jl#49

It builds in Julia 1.6 - ubuntu, but there is still an HTTP problem.

get: Error During Test at /home/runner/work/GADM.jl/GADM.jl/test/GADM.jl:34
  Got exception outside of a @test
  HTTP.Exceptions.RequestError(HTTP.Messages.Request:
  """
  GET /data/gadm3.6/gpkg/gadm36_SUR_gpkg.zip HTTP/1.1
  Host: biogeo.ucdavis.edu
  Accept: */*
  User-Agent: HTTP.jl/1.6.7
  Content-Length: 0

ErickChacon · 2023-05-04T01:27:21Z

I added tests for writing to .shp and .geojson for points, lines, polygons and multipolygons. I consider this PR completed. All tests are passing in my linux machine.

juliohm · 2023-05-04T01:34:57Z

test/runtests.jl

+    writedir = joinpath(tempdir(), "geotables")
+    mkdir(writedir)


I think we should write to the datadir that is already defined. Can we avoid creating a new directory?

codecov-commenter · 2023-05-04T02:13:15Z

Codecov Report

Merging #15 (37fa290) into master (cea71c8) will decrease coverage by 1.18%.
The diff coverage is 86.31%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master      #15      +/-   ##
==========================================
- Coverage   88.57%   87.39%   -1.18%     
==========================================
  Files           3        3              
  Lines          70      119      +49     
==========================================
+ Hits           62      104      +42     
- Misses          8       15       +7

Impacted Files	Coverage Δ
src/conversion.jl	`83.87% <83.87%> (+12.44%)`	⬆️
src/geotable.jl	`88.57% <86.66%> (-5.37%)`	⬇️
src/GeoTables.jl	`95.45% <94.44%> (-4.55%)`	⬇️

juliohm · 2023-05-04T12:49:14Z

Thank you @ErickChacon! ❤️

write to shapefile

0adb42d