Opened 13 years ago

Closed 12 years ago

#1628 closed defect (fixed)

[PATCH] OGR support of empty wkt

Reported by: hobu Owned by: Even Rouault
Priority: normal Milestone: 1.6.0
Component: OGR_SF Version: 1.4.1
Severity: normal Keywords:
Cc: warmerdam

Description

If I understand it correctly, the WKT spec http://portal.opengeospatial.org/files/?artifact_id=13227 says we should be able to have empty geometries. This doesn't seem to be the case with OGR, however.

>>> import ogr
>>> wkt = 'POINT EMPTY'
>>> g = ogr.CreateGeometryFromWkt(wkt)
>>> g
<ogr.Geometry; proxy of <Swig Object of type 'OGRGeometryShadow *' at 0x304100> >
>>> print g
POINT (0 0)

POINT (0 0) is not an empty geometry. Is this just a limitation (inability to have empty geoms) of OGR and is there any expectation that it might be fixed some day?

Attachments (2)

gdal_svn_trunk_ogr_point_polygon_is_empty.patch (3.4 KB) - added by Even Rouault 12 years ago.
gdal_svn_trunk_autotest_ogr_geom_py.patch (5.5 KB) - added by Even Rouault 12 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 13 years ago by warmerdam

Cc: warmerdam added
Component: defaultOGR_SF
Milestone: 1.5.0
Owner: changed from warmerdam to Mateusz Łoskot

This is a known issue with the OGRPoint. I had previously felt there was no obvious way to mark a point object as empty, but today it occured to me we could set the CoordinateDimension? field to zero as a special marker for empty. I will queue this change up for 1.5.0 for Mateusz.

Mateusz,

We will need to be pretty careful that this doesn't screw anything else up.

comment:2 Changed 13 years ago by Mateusz Łoskot

Status: newassigned

comment:3 Changed 13 years ago by Mateusz Łoskot

This issue is the reason of failing OGR test case on telascience buildbot slave:

  TEST: ogr_geom_empty ... fail
    IsEmpty returning false for an empty geometry

...

 ------------ Failures ------------
Script: ogr/ogr_geom.py
  TEST: ogr_geom_empty ... fail
    IsEmpty returning false for an empty geometry
 ----------------------------------

comment:4 Changed 13 years ago by warmerdam

Milestone: 1.5.0

Deferred till 1.6 or later.

comment:5 Changed 12 years ago by Even Rouault

Summary: OGR support of empty wkt[PATCH] OGR support of empty wkt

Attached a patch that implements Frank's idea. I've also added tests to ogr_geom.py for empty geometries, and I discovered a problem with 'POLYGON EMPTY'. GEOS (GEOS 2.2.3) emits a warning about truncated WKB (since the IsEmpty?() method uses GEOS). So I overloaded IsEmpty? for OGRGeometry too.

Changed 12 years ago by Even Rouault

Changed 12 years ago by Even Rouault

comment:6 Changed 12 years ago by Even Rouault

Milestone: 1.6.0

comment:7 Changed 12 years ago by Even Rouault

As you may notice, I finally removed the added tests from ogr_geom.py, as I saw that there was an existing ogr_wktempty.py, where I fixed the POINT EMPTY case, and added a call to IsEmpty?() for each case.

comment:8 Changed 12 years ago by Mateusz Łoskot

Even, I'm not sure if Frank would agree, but I'd suggest to submit your patches directly instead of submitting them to a ticket. You have the commit access so you should be able to submit your patches. IMO it would simplify the process and avoid situation that both of us are working on the same ticket.

If there is a ticket you are going to work on, I'd suggest following steps:

  1. Reassign it to yourself
  2. Fix and commit patches
  3. Close ticket as fixed

After step 1, I will be notified that you've taken it over.

Frank, what do you think?

comment:9 Changed 12 years ago by warmerdam

I agree that assigning a ticket to yourself should be done before proceeding with work on it (and noting in the ticket at that time that you are looking into it).

Attaching a patch to the ticket is ok if you are uncomfortable applying it without some further review, but it does increase overhead and it is not uncommon for such patches to sit around for months unreviewed. So if you are just a little uncertain it is likely better to apply the patch and explain any concerns in the ticket - as well as adding any likely reviewers/commenters to the cc: list.

comment:10 Changed 12 years ago by Even Rouault

Owner: changed from Mateusz Łoskot to Even Rouault
Status: assignednew

Understood on the way of proceeding with tickets.

Patch commited in trunk in r13909 and test updated in r13910

comment:11 Changed 12 years ago by warmerdam

Even,

Is this ticket ready to close?

comment:12 Changed 12 years ago by Even Rouault

Resolution: fixed
Status: newclosed

Yes, closing

Note: See TracTickets for help on using tickets.