xbikuna / nettopologysuite

Automatically exported from code.google.com/p/nettopologysuite
0 stars 0 forks source link

Shapefile read fails to a read a shapefile if the shapefile contains an element with 0 points #144

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago

1. Create a polygon shapefile which has some valid polygons, and one polgon 
which has no points. I received a file from a client and will submit that one 
if I get clearance to use it.

2. Try to read the shapefile with NetTopology. 

3. The entire read will fail, since an exception is tossed in the polygon 
handler.

What is the expected output? What do you see instead?

I expect to be able to read all the polygons even if one does not have points. 
ArcGIS 9.1 can read the file in question.

Seems like we have the same issue for polylines.

I have created a patch (attached) which solves this issue for both polygons and 
lines.

Original issue reported on code.google.com by tbe...@gmail.com on 22 Mar 2013 at 11:04

Attachments:

GoogleCodeExporter commented 8 years ago

Original comment by tbe...@gmail.com on 22 Mar 2013 at 11:57

GoogleCodeExporter commented 8 years ago
@tbenum: This does not prevent "POLYGON EMPTY" or "MULTILINESTRING EMPTY" 
geometries, does it?

Original comment by felix.ob...@netcologne.de on 2 Apr 2013 at 9:33

GoogleCodeExporter commented 8 years ago
That is a good point - it might - we *could* potentially have an issue here
(but I don't think we do). If POLYGON EMPTY worked before, my change would
not break that, as the code I changed would toss an exception if you called
it with a empty part. But, it could have either already been broken, OR it
could be that we now introduce a unwanted side effect:

Let me explain:

The effect of my change is that empty parts will be skipped - instead of
trying to add an empty part to a multi part polygon/line. I am not sure if
it is a valid case to have a polygon with 2 parts with some points and 1
part with no points? If we want to preserve the latter case, we should
modify my fix (see further down).

If I recall correctly, the underlying issue was that this code caused an
exception to be tossed:

       public static bool IsCCW(Coordinate[] ring) 
        {
            // # of points without closing endpoint
            int nPts = ring.Length - 1;

            // sanity check
            if (nPts < 3)
                throw new ArgumentException("Ring has fewer than 4 points,
so orientation cannot be determined");

A better fix for this bug if we want to carry empty elements along could be
to modify the ring.IsCCW test to test for:

If (!ring.IsEmpty && ring.IsCCW)

But I am not sure if it makes sense to have empty elements in there? I
thought it would not - that is why I changed the code to skip them.

Let me know if you want me to change the fix to the test above instead.

Without testing this I would guess that a purely EMPTY POLYGON should work -
since the sequences array will be empty and the problematic code will never
be called. The problem occurs when you have a polygon with some valid parts,
and some empty parts.

Trond
p.s. As for polylines - it might be that empty polylines will already work
and that we should not modify that code at all. I did not consider an empty
linestring element valid and thought it was safest to skip it. But It could
be that it will work without and we should not modify that code.

Original comment by tbe...@gmail.com on 2 Apr 2013 at 10:16

GoogleCodeExporter commented 8 years ago
I'll prepare some unit tests

Original comment by felix.ob...@netcologne.de on 2 Apr 2013 at 11:27

GoogleCodeExporter commented 8 years ago
@trond, I've applied your patch and a fix for handling empty shapes. Could you 
please verify

Original comment by felix.ob...@netcologne.de on 24 Jun 2013 at 1:55