Opened 5 years ago

Closed 5 years ago

#31142 closed Bug (fixed)

MakeValid() polygon to multi-polygon raises error in GEOS C function "GEOSGetNumInteriorRings_r".

Reported by: Eran Keydar Owned by: Sergey Fedoseev
Component: GIS Version: dev
Severity: Normal Keywords:
Cc: Sergey Fedoseev Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have model with Polygon field.
Using MakeValid on the polygon field returns MultiPolygon, but the resulting geom is broken.

This worked on 1.11 and failed on 2.2

A simple solution is to make the fix on MultiPolygon field (with single polygon).

This is the model (app1/models.py):

from django.contrib.gis.db import models
class WithPolygon(models.Model):
    geom = models.PolygonField()

Then I run the following code:

def fix_polygon():
    import django
    from django.contrib.gis.db.models.functions import MakeValid
    from django.contrib.gis.geos import MultiPolygon, Polygon
    from app1.models import WithPolygon
    print('version = {}'.format(django.__version__))
   # create disjoint rings
    r1 = [[0,0],[100,0],[100,100],[0, 100], [0,0]]
    r2 = [[x+1000,y+1000] for x,y in r1]
   #  create polygon with two rings
    p1 = Polygon(r1, r2)
    print('p1.valid  = {}'.format(p1.valid))  # False
    print('p1.valid_reason =  {}'.format(p1.valid_reason)) # Hole lies outside shell[1000 1000]
    wp1 = WithPolygon.objects.create(geom=p1)
    wp1_with_fix = WithPolygon.objects.annotate(v=MakeValid('geom')).filter(id=wp1.id)[0]
    mp = wp1_with_fix.v
    print('mp.geom_type = {}'.format(mp.geom_type)) # MultiPolygon
    print('mp.__class__ = {}'.format(mp.__class__)) # returns Polygon in 2.2 MultiPolygon in 1.11
    print(len(mp))  # GESOEXception in 2.2, (In 1.11 prints 2)

Outputs are attached.

Attachments (2)

output_111.txt (259 bytes ) - added by Eran Keydar 5 years ago.
output_22.txt (2.8 KB ) - added by Eran Keydar 5 years ago.

Download all attachments as: .zip

Change History (9)

by Eran Keydar, 5 years ago

Attachment: output_111.txt added

by Eran Keydar, 5 years ago

Attachment: output_22.txt added

comment:1 by Carlton Gibson, 5 years ago

Hi Eran,

Can I ask you to double check that the only difference is your Django version here? Most of the functions, including MakeValid are just wrappers around the DB functionality or the underlying C GEOS library.

I can´t see a relevant change from the provided stack trace between 1.11 and 2.2 (git difftool 1.11 2.2 -- django/contrib/gis).

We have a test case for MakeValid here. It's similar, but not exactly equivalent to your case.

Version 0, edited 5 years ago by Carlton Gibson (next)

comment:2 by Eran Keydar, 5 years ago

The only difference is the Django version.

The difference is that in test suite, a polygon is fixed to polygon, while in my case a polygon is fixed into multi polygon.

I've added the following test case and then ran it with the django git, with branch stable/1.11.x stable/2.2.x and mater.
I've used postgis as the database.
It failed on both master and 2.2.x but passed on 1.11.x


    @skipUnlessDBFeature("has_MakeValid_function")
    def test_make_valid2(self):
        invalid_geom = Polygon(
            ((0,0),(0,1),(1,1),(1,0),(0,0)),
            ((10,0),(10,1),(11,1),(11,0),(10,0)),
        )
        State.objects.create(name='invalid', poly=invalid_geom)
        invalid = State.objects.filter(name='invalid').annotate(repaired=functions.MakeValid('poly')).first()
        self.assertIs(invalid.repaired.valid, True)
        self.assertEqual(invalid.repaired.geom_type,'MultiPolygon')
        self.assertEqual(len(invalid.repaired), 2)
Last edited 5 years ago by Eran Keydar (previous) (diff)

comment:3 by Mariusz Felisiak, 5 years ago

Cc: Sergey Fedoseev added
Component: UncategorizedGIS
Summary: MakeValid for polygon returns "broken" MultiPolygonMakeValid() polygon to multi-polygon raises error in GEOS C function "GEOSGetNumInteriorRings_r".
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug
Version: 2.2master

Thanks for tests.

Regression in 2ef4b4795e29be8c33a6de9cc0c05b59025d13a5.

comment:4 by Sergey Fedoseev, 5 years ago

Owner: changed from nobody to Sergey Fedoseev
Status: newassigned

comment:5 by Sergey Fedoseev, 5 years ago

Has patch: set

comment:6 by Claude Paroz, 5 years ago

Triage Stage: AcceptedReady for checkin

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In a920c0b8:

Fixed #31142 -- Fixed MakeValid.output_field when geometry type is changed.

Regression in 2ef4b4795e29be8c33a6de9cc0c05b59025d13a5.

Note: See TracTickets for help on using tickets.
Back to Top