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)
Change History (5)
by , 39 hours ago
| Attachment: | geoserrors.patch added |
|---|
comment:1 by , 33 hours ago
| Cc: | added |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
| Type: | Uncategorized → Cleanup/optimization |
comment:2 by , 21 hours ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
comment:3 by , 21 hours ago
| Owner: | changed from to |
|---|
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 , 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.
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!