Opened 39 hours ago

Last modified 20 hours ago

#36849 assigned Cleanup/optimization

Moved GEOS Error Messages to Exceptions

Reported by: David Smith Owned by: David Smith
Component: GIS Version: 6.0
Severity: Normal Keywords:
Cc: Claude Paroz Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Capturing GEOS error messages can be a challenge as they are raised in the underlyingn C extension. In #17959 GEOS error messages were moved to logs, see 53c8b2c0c52cf999b644184bfe51e9f59d89286e.

Here is an example of the current output. Firstly an error is logged and later you have an Exception.

>>> from django.contrib.gis.geos import GEOSGeometry
>>> GEOSGeometry("POINT (5, 23)")
GEOS_ERROR: ParseException: Expected number but encountered ','

Traceback (most recent call last):
  File "<python-input-1>", line 1, in <module>
    GEOSGeometry("POINT (5, 23)")
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
  File "C:\Users\smith\projects\django\django\contrib\gis\geos\geometry.py", line 766, in __init__
    g = self._from_wkt(force_bytes(wkt_m["wkt"]))
  File "C:\Users\smith\projects\django\django\contrib\gis\geos\geometry.py", line 139, in _from_wkt
    return wkt_r().read(wkt)
           ~~~~~~~~~~~~^^^^^
  File "C:\Users\smith\projects\django\django\contrib\gis\geos\prototypes\io.py", line 160, in read
    return wkt_reader_read(self.ptr, force_bytes(wkt))
  File "C:\Users\smith\projects\django\django\contrib\gis\geos\libgeos.py", line 154, in __call__
    return self.func(*args)
           ~~~~~~~~~^^^^^^^
  File "C:\Users\smith\projects\django\django\contrib\gis\geos\prototypes\threadsafe.py", line 47, in __call__
    return self.cfunc(self.thread_context.handle.ptr, *args)
           ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\smith\projects\django\django\contrib\gis\geos\prototypes\errcheck.py", line 35, in check_geom
    raise GEOSException(
    ...<2 lines>...
    )
django.contrib.gis.geos.error.GEOSException: Error encountered checking Geometry returned from GEOS C function "GEOSWKTReader_read_r".

I propose to move the GEOS error message to the exception message.

>>> from django.contrib.gis.geos import GEOSGeometry
>>> GEOSGeometry("POINT (5, 23)")
Traceback (most recent call last):
  File "<python-input-1>", line 1, in <module>
    GEOSGeometry("POINT (5, 23)")
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
  File "C:\Users\smith\projects\django\django\contrib\gis\geos\geometry.py", line 766, in __init__
    g = self._from_wkt(force_bytes(wkt_m["wkt"]))
  File "C:\Users\smith\projects\django\django\contrib\gis\geos\geometry.py", line 139, in _from_wkt
    return wkt_r().read(wkt)
           ~~~~~~~~~~~~^^^^^
  File "C:\Users\smith\projects\django\django\contrib\gis\geos\prototypes\io.py", line 160, in read
    return wkt_reader_read(self.ptr, force_bytes(wkt))
  File "C:\Users\smith\projects\django\django\contrib\gis\geos\libgeos.py", line 155, in __call__
    return self.func(*args)
           ~~~~~~~~~^^^^^^^
  File "C:\Users\smith\projects\django\django\contrib\gis\geos\prototypes\threadsafe.py", line 48, in __call__
    return self.cfunc(self.thread_context.handle.ptr, *args)
           ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\smith\projects\django\django\contrib\gis\geos\prototypes\errcheck.py", line 47, in check_geom
    raise GEOSException(error_msg)
django.contrib.gis.geos.error.GEOSException: Error encountered checking Geometry returned from GEOS C function "GEOSWKTReader_read_r". ParseException: Expected number but encountered ','
>>> 

This would make the error appear inline with the exception rather than as a separate message. I also found that the Django test suite doesn't have any logging output configured by default so these messages can be difficult to spot.

I'll attach a tentative patch to show how this could be achieved.

Attachments (1)

geoserrors.patch (14.7 KB ) - added by David Smith 39 hours ago.

Download all attachments as: .zip

Change History (5)

by David Smith, 39 hours ago

Attachment: geoserrors.patch added

comment:1 by Natalia Bidart, 33 hours ago

Cc: Claude Paroz added
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

Thank you David for creating this ticket. I think it's a good idea and a nice improvement. I thought I saw a ticket for this before, but I can't find it, so accepting this one.

Looking forward to the PR!

comment:2 by Pravin, 21 hours ago

Owner: set to Pravin
Status: newassigned

comment:3 by David Smith, 21 hours ago

Owner: changed from Pravin to David Smith

Hi Pravin - I've already got a well developed patch. I'll assign this to myself with the aim being to avoid duplicating work. A review once I have completed it would be appreciated if you have interest in this.

comment:4 by Pravin, 20 hours ago

yeah no worries, David. I assigned to me bcs there was no owner assigned. Your patch look pretty much solid. and I was taking reference from your patch. you did right move to notify me immediately.
Anyway thanks.

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