Opened 5 years ago

Last modified 5 years ago

#21995 closed defect

Fix comparison operators of PoorManMap — at Version 10

Reported by: saraedum Owned by:
Priority: minor Milestone: sage-7.5
Component: categories Keywords:
Cc: Merged in:
Authors: Julian Rüth Reviewers:
Report Upstream: N/A Work issues:
Branch: u/saraedum/fix_comparison_operators_of_poormanmap (Commits, GitHub, GitLab) Commit: 554bae8ecb9e67741de85f6b71aca09a173a1519
Dependencies: Stopgaps:

Status badges

Description (last modified by saraedum)

PoorManMap? currently only implements __eq__ but not __hash__ nor __ne__.

This ticket implements _richcmp_ and __hash__ instead of __eq__ and makes _richcmp_ not rely on comparison of __dict__ which does not work anymore when introducing @cached_methods in PoorManMap?, see #21894.

Change History (10)

comment:1 Changed 5 years ago by saraedum

  • Branch set to u/saraedum/fix_comparison_operators_of_poormanmap

comment:2 Changed 5 years ago by saraedum

  • Commit set to 0d0f55d089dd299bfd36b6b441f142e2fbdd8fd8
  • Description modified (diff)

New commits:

0d0f55dImplement _richcmp_ and __hash__ for PoorManMap

comment:3 Changed 5 years ago by saraedum

  • Status changed from new to needs_review

I still have to wait for my sage to build to check that this actually works.

comment:4 Changed 5 years ago by saraedum

  • Status changed from needs_review to needs_work

I just realized that PoorManMap? is inherited from PoorManMapCompose?. Got to fix that one as well.

comment:5 Changed 5 years ago by git

  • Commit changed from 0d0f55d089dd299bfd36b6b441f142e2fbdd8fd8 to 3f20aaa44c15ff93b816fe787742ad60d390f37c

Branch pushed to git repo; I updated commit sha1. New commits:

40a6d5ba PoorManMap is unequal to anything else
3f20aaaMake PoorManComposeMap correctly inherit from PoorManMap

comment:6 Changed 5 years ago by saraedum

  • Status changed from needs_work to needs_review

comment:7 Changed 5 years ago by saraedum

  • Status changed from needs_review to needs_work

Xavier Caruso proposed to simplify this by integrating the compose map logic into PoorManMap.

comment:8 Changed 5 years ago by git

  • Commit changed from 3f20aaa44c15ff93b816fe787742ad60d390f37c to 554bae8ecb9e67741de85f6b71aca09a173a1519

Branch pushed to git repo; I updated commit sha1. New commits:

554bae8Removed PoorManComposeMap

comment:9 Changed 5 years ago by saraedum

  • Status changed from needs_work to needs_review

comment:10 Changed 5 years ago by saraedum

  • Description modified (diff)
Note: See TracTickets for help on using tickets.