Bug report #8775

PyQGIS QgsPoint has a __hash__ function, even though it is mutable

Added by J. Dugge over 10 years ago. Updated about 5 years ago.

Status:Closed
Priority:Normal
Assignee:-
Category:Python plugins
Affected QGIS version:master Regression?:No
Operating System: Easy fix?:No
Pull Request or Patch supplied:No Resolution:end of life
Crashes QGIS or corrupts data:No Copied to github as #:17481

Description

The QgsPoint class in PyQGIS has a (automatically generated?) __hash__ function, which returns a hash value that does not depend on the coordinates of the point. This leads to the inconsistent behaviour that two points that are equal according to QgsPoint.__eq__ do not have the same hash value, which causes problems with functions that rely on proper __hash__ behaviour, like set.

To reproduce this, load a polygon layer and run the following in the Python console:

provider = iface.activeLayer().dataProvider()
for f in provider.getFeatures():
  feature = f
points = f.geometry().asPolygon()[0]

points[0] == points[1]
# Returns True, the first and last points in a polygon are identical

set(points)
# The first/last point appears twice in the set, even though it should only appear once according to the equality

To fix this, the __hash__ function in QgsPoint should be removed (e.g. by setting QgsPoint.__hash__ = None), which will raise TypeError: unhashable type: 'QgsPoint' when set is used with a list of QgsPoint objects.

Associated revisions

Revision 44b77671
Added by Matthias Kuhn over 10 years ago

Create hash method for QgsPoint (Fix #8775)

History

#2 Updated by Matthias Kuhn over 10 years ago

  • Status changed from Open to Closed

#3 Updated by J. Dugge over 10 years ago

Thanks for the quick reaction!

I think the changeset doesn't actually fix the issue though: QgsPoint is a mutable type (its value can be changed using `setX()`, for instance), and as such, it mustn't have a __hash__() function (see http://docs.python.org/2/glossary.html#term-hashable)

Consider the following to see why the new implementation is problematic:

a = QgsPoint(0,0)
b = QgsPoint(1,1)
c = set([a,b])

print c
# correctly returns [(0,0),(1,1)]

a.set(1,1)
print c
# returns [(1,1),(1,1)], which is incorrect

The proper way to fix this is to remove the __hash__ function by setting __hash__ = None, which explicitly marks the class as the mutable and unhashable type it is (http://docs.python.org/2/reference/datamodel.html#object.__hash__) so set operations (which don't work with mutable types) are disabled.

#4 Updated by J. Dugge over 10 years ago

  • Status changed from Closed to Reopened

#5 Updated by Giovanni Manghi almost 7 years ago

  • Regression? set to No
  • Easy fix? set to No

#6 Updated by Giovanni Manghi about 5 years ago

  • Resolution set to end of life
  • Status changed from Reopened to Closed

Also available in: Atom PDF