[Django] #36849: Moved GEOS Error Messages to Exceptions

[Django]

#36849: Moved GEOS Error Messages to Exceptions
-----------------------------+-----------------------------------------
Reporter: David Smith | Type: Uncategorized
Status: new | Component: GIS
Version: 6.0 | Severity: Normal
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+-----------------------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/36849>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36849: Moved GEOS Error Messages to Exceptions

-------------------------------+--------------------------------------
Reporter: David Smith | Owner: (none)

Type: Uncategorized | Status: new
Component: GIS | Version: 6.0

Severity: Normal | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------+--------------------------------------
Changes (by David Smith):

* Attachment "geoserrors.patch" added.

[Django]

#36849: Moved GEOS Error Messages to Exceptions

--------------------------------------+------------------------------------

Reporter: David Smith | Owner: (none)

Type: Cleanup/optimization | Status: new

Component: GIS | Version: 6.0
Severity: Normal | Resolution:

Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

--------------------------------------+------------------------------------
Changes (by Natalia Bidart):

* cc: Claude Paroz (added)
* stage: Unreviewed => Accepted
* type: Uncategorized => Cleanup/optimization

Comment:

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!
--
Ticket URL: <https://code.djangoproject.com/ticket/36849#comment:1>

[Django]

#36849: Moved GEOS Error Messages to Exceptions
--------------------------------------+------------------------------------

Reporter: David Smith | Owner: Pravin
Type: Cleanup/optimization | Status: assigned

Component: GIS | Version: 6.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------

Changes (by Pravin):

* owner: (none) => Pravin
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36849#comment:2>

[Django]

#36849: Moved GEOS Error Messages to Exceptions

-------------------------------------+-------------------------------------
Reporter: David Smith | Owner: David
Type: | Smith

Cleanup/optimization | Status: assigned
Component: GIS | Version: 6.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by David Smith):

* owner: Pravin => David Smith

Comment:

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.
--
Ticket URL: <https://code.djangoproject.com/ticket/36849#comment:3>

[Django]

#36849: Moved GEOS Error Messages to Exceptions
-------------------------------------+-------------------------------------
Reporter: David Smith | Owner: David
Type: | Smith
Cleanup/optimization | Status: assigned
Component: GIS | Version: 6.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Pravin):

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.
--
Ticket URL: <https://code.djangoproject.com/ticket/36849#comment:4>

[Django]

#36849: Moved GEOS Error Messages to Exceptions
-------------------------------------+-------------------------------------
Reporter: David Smith | Owner: David
Type: | Smith
Cleanup/optimization | Status: assigned
Component: GIS | Version: 6.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by David Smith):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36849#comment:5>