Opened 2 years ago
Closed 2 years 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 2 years ago by
- Branch set to public/manifold/continous_map_hash
- Commit set to 83dcbb1e363942f044fcb9a78d05c7bf8029be0b
comment:2 Changed 2 years ago by
- Cc tscrim embray chapoton added
- Status changed from new to needs_review
comment:3 follow-up: ↓ 4 Changed 2 years ago by
- 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 2 years ago by
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 2 years ago by
- Branch changed from public/manifold/continous_map_hash to 83dcbb1e363942f044fcb9a78d05c7bf8029be0b
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
Add __hash__ to ContinousMap