Opened 6 months ago

Closed 6 months ago

#25502 closed defect (fixed)

py3: implement __hash__ method in ContinuousMap

Reported by: egourgoulhon Owned by:
Priority: major Milestone: sage-8.3
Component: geometry Keywords: python3, topological manifold
Cc: tscrim, embray, chapoton Merged in:
Authors: Eric Gourgoulhon Reviewers: Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: 83dcbb1 (Commits) Commit: 83dcbb1e363942f044fcb9a78d05c7bf8029be0b
Dependencies: Stopgaps:

Description

ContinuousMap defines its own __eq__ method, while inheritating from Morphism.__hash__. Now, as pointed out in #24551, Python3 breaks the inheritance of __hash__ when __eq__ is redefined. This ticket fixes this by implementing ContinousMap.__hash__. Moreover, contrary to Morphism.__hash__, the new method does not rely on repr() and guarantees that two continous maps that compare equal have the same hash.

Note that there is a proposal at #25393 to remove Morphism.__hash__.

ContinousMap (more precisely the derived class DiffMap) needs to be hashable because DiffMap objects serve as keys in the dictionary of vector field modules on a manifold, a module of vector fields on a differentiable manifold being essentially defined by a differentiable map (with the important special case of the identity map; see here for details).

Change History (5)

comment:1 Changed 6 months ago by egourgoulhon

  • Branch set to public/manifold/continous_map_hash
  • Commit set to 83dcbb1e363942f044fcb9a78d05c7bf8029be0b

New commits:

83dcbb1Add __hash__ to ContinousMap

comment:2 Changed 6 months ago by egourgoulhon

  • Cc tscrim embray chapoton added
  • Status changed from new to needs_review

comment:3 follow-up: Changed 6 months ago by chapoton

  • Reviewers set to Frédéric Chapoton
  • Status changed from needs_review to positive_review

does seem to be a very rough hash... But it will do the job for the moment

comment:4 in reply to: ↑ 3 Changed 6 months ago by egourgoulhon

Replying to chapoton:

does seem to be a very rough hash... But it will do the job for the moment

Yes it's pretty rough, but there should be no loss of efficiency, since continuous maps are only used as keys in the dictionary DifferentiableManifold._vector_field_modules, which is of very small size; most of the time, it has a single element (the continuous map being then merely the identity map).

comment:5 Changed 6 months ago by vbraun

  • Branch changed from public/manifold/continous_map_hash to 83dcbb1e363942f044fcb9a78d05c7bf8029be0b
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.