Shapefile.jl icon indicating copy to clipboard operation
Shapefile.jl copied to clipboard

[sd/geometrybasics] Simplify Polygon code and add tests

Open greimel opened this issue 4 years ago • 1 comments

The test cases are adapted from this file in pyshp (commit e5407f38c571029721d7aaa9cabca2aa7ecd9019)

The main tests are shown below.

image

image

Right now the tests are in a Pluto notebook. If desired I can adapt the notebook to make it includable from runtests.jl (with or without the plots).

This addresses the first part of https://github.com/JuliaGeo/Shapefile.jl/pull/39#issuecomment-805713979

cc @SimonDanisch @Sov-trotter

greimel avatar Aug 04 '21 14:08 greimel

All tests pass locally. I have not yet integrated the new tests into the test suite. You will see that the old polygon tests yield a warning because they do not follow the official specification. (This is in line with pyshp: read correctly, but warn.)

I'll start using this branch now for some work, I'll report here if something needs to be fixed.

In the meantime, this is ready for review. Please let me know

  1. if you want me to include("notebook.jl") or just add the plain tests to the test file.
  2. if I should remove the old polygon tests (since they are incorrectly specified)

greimel avatar Aug 05 '21 11:08 greimel

It would be good to use these tests, but the current (newer) implementation is lazy over rings which is much faster for many applications than separating out the polygons. I'm not sure this is possible with GeometryBasics.jl objects, so closing this PR.

rafaqz avatar Dec 13 '22 17:12 rafaqz