[Django] #26920: GEOSGeometry objects compare equal despite different SRID
Django
-------------------------------+---------------------------------
Reporter: das-g | Owner: nobody
Type: Uncategorized | Status: new
Component: GIS | Version: 1.9
Severity: Normal | Keywords: geodjango, equality
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+---------------------------------
Checking equality of two `django.contrib.gis.geos.Point` objects (and
presumably any other `GEOSGeometry` objects of equal type) does not take
the geometries' SRIDs into consideration:
{{{#!python
from django.contrib.gis.geos import Point
p0 = Point(5, 23) # SRID implicitly None
p1 = Point(5, 23, srid=4326)
p2 = Point(5, 23, srid=32632)
assert p0.srid != p1.srid # passes
assert p0.srid != p2.srid # passes
assert p1.srid != p2.srid # passes
assert p0 != p1 # AssertionError
assert p0 != p2 # AssertionError
assert p1 != p2 # AssertionError
}}}
The semantic of coordinates depends completely on the spatial reference
system. The same numbers refer to completely different positions when
interpreted in different coordinate systems. Therefore, I think the
geometries should compare unequal if both have an SRID set and those SRIDs
are different. Not sure what should happen when comparing geometries where
exactly one of them has an SRID.
If the currently observed behaviour is the intended one (or if it is
retained for backwards compatibility reasons), the documentation should
point that out.
[https://docs.djangoproject.com/en/1.9/ref/contrib/gis/geos/#geometries-
are-pythonic It already points out that the "Equality operator doesn’t
check spatial equality"], but to me that only made clear that spatial
equality does not imply `GEOSGeometry` object equality, but not that
neither does `GEOSGeometry` object equality imply spatial equality. Also,
[https://docs.djangoproject.com/en/1.9/ref/contrib/gis/geos/#django.contrib.gis.geos.GEOSGeometry.equals
equals()], which is mentioned in that admonition as an alternative doesn't
seem to care about SRID, either:
{{{#!python
assert not p0.equals(p1) # AssertionError
assert not p0.equals(p2) # AssertionError
assert not p1.equals(p2) # AssertionError
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26920>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: das-g | Owner: nobody
Type: Uncategorized | Status: new
Component: GIS | Version: 1.9
Keywords: geodjango, equality | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by das-g):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Old description:
New description:
equals()], which is mentioned in that admonition as an alternative,
doesn't seem to care about SRID, either:
{{{#!python
assert not p0.equals(p1) # AssertionError
assert not p0.equals(p2) # AssertionError
assert not p1.equals(p2) # AssertionError
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/26920#comment:1>
Django
Reporter: das-g | Owner: nobody
Type: Uncategorized | Status: new
Component: GIS | Version: 1.9
Keywords: geodjango, equality | Triage Stage:
| Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by das-g):
Like with #25342, the behaviour of the `==` operator reflects that of the
[https://github.com/libgeos/libgeos/blob/3d15551143c93f6916cb5f2f2a40cf9971975f89/include/geos/geom/Point.h#L128
underlying `equalsExact` method] (implementation for `Point`
[https://github.com/libgeos/libgeos/blob/3d15551143c93f6916cb5f2f2a40cf9971975f89/src/geom/Point.cpp#L216
here]; each geometry type seems to have its own implementation), but I'm
not sure it ''should'' reflect that behaviour in this regard (ignoring the
SRID), too.
--
Ticket URL: <https://code.djangoproject.com/ticket/26920#comment:2>
Django
Reporter: das-g | Owner: nobody
Type: Uncategorized | Status: new
Component: GIS | Version: 1.9
Keywords: geodjango, equality | Triage Stage:
| Unreviewed
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* cc: sir-sigurd, claudep (added)
Comment:
Claude and/or Sergey, can you advise?
--
Ticket URL: <https://code.djangoproject.com/ticket/26920#comment:3>
Django
Reporter: das-g | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: GIS | Version: 1.9
Severity: Normal | Resolution:
Keywords: geodjango, equality | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* type: Uncategorized => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
Not sure about the appropriate resolution (maybe start a discussion on the
[https://groups.google.com/forum/#!forum/geodjango geodjango mailing list]
if needed). Accepting the ticket at least on a documentation basis if
there's no consensus to make code changes.
--
Ticket URL: <https://code.djangoproject.com/ticket/26920#comment:4>
Django
Reporter: Raphael Das Gupta | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: GIS | Version: master
Severity: Normal | Resolution:
Keywords: geodjango, equality | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jackie Leng):
* cc: jackie.leng@… (added)
* has_patch: 0 => 1
* version: 1.9 => master
Comment:
I've submitted a patch for this issue:
https://github.com/django/django/pull/7507. With this patch the equality
check will take into account the srid (both for GEOSGeometry and EWKT).
--
Ticket URL: <https://code.djangoproject.com/ticket/26920#comment:5>
Django
Reporter: Raphael Das Gupta | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: GIS | Version: master
Severity: Normal | Resolution:
Keywords: geodjango, equality | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
Comment:
There are some test failures on MySQL. Even though this might be
considered a bug fix, I wonder if it's a good candidate for mention in the
release notes as a possible backwards-incompatible change?
--
Ticket URL: <https://code.djangoproject.com/ticket/26920#comment:6>
Django
Reporter: Raphael Das Gupta | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: GIS | Version: master
Severity: Normal | Resolution:
Keywords: geodjango, equality | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jackie Leng):
* needs_better_patch: 1 => 0
Comment:
Added requested changes, but there is failure on Oracle.
--
Ticket URL: <https://code.djangoproject.com/ticket/26920#comment:7>
Django
Reporter: Raphael Das Gupta | Owner: nobody
Cleanup/optimization |
Component: GIS | Version: master
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* stage: Accepted => Ready for checkin
Comment:
Looks good, pending a +1 from a GIS expert.
--
Ticket URL: <https://code.djangoproject.com/ticket/26920#comment:8>
Django
Reporter: Raphael Das Gupta | Owner: nobody
Cleanup/optimization |
Component: GIS | Version: master
Keywords: geodjango, equality | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz <claude@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"50613d957a853f83310a03e0f300bbf568f1f65b" 50613d95]:
{{{
#!CommitTicketReference repository=""
revision="50613d957a853f83310a03e0f300bbf568f1f65b"
Fixed #26920 -- Made GEOSGeometry equality check consider the srid
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26920#comment:9>