Opened 3 years ago

Closed 3 years ago

#25935 closed defect (fixed)

Make __eq__ and __hash__ of AbelianGroup_class consistent

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-8.4
Component: group theory Keywords:
Cc: jhpalmieri Merged in:
Authors: John Palmieri Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: 6c25916 (Commits, GitHub, GitLab) Commit: 6c25916cdca7fbfc6f7d89b3bc0c5d04ebea7fdd
Dependencies: Stopgaps:

Status badges

Description

AbelianGroup_class inherits its __hash__ from UniqueRepresentation but it has a different __eq__: 

__eq__ = is_isomorphic

Either we should remove that special __eq__ or we should make __hash__ consistent with __eq__.

Change History (8)

comment:1 Changed 3 years ago by jhpalmieri

  • Branch set to u/jhpalmieri/abelian_gp_eq

comment:2 Changed 3 years ago by jhpalmieri

  • Commit set to 6c25916cdca7fbfc6f7d89b3bc0c5d04ebea7fdd

Well, I removed __eq__ and __ne__ and there were only a few doctest failures. So I say we go this route.

New commits:

​6c25916trac 25935: remove __eq__ for AbelianGroup_class: it should inherit __eq__

comment:3 Changed 3 years ago by jhpalmieri

  • Authors set to John Palmieri
  • Status changed from new to needs_review

I'll mark this as "needs review", but feel free to disagree with the approach.

comment:4 Changed 3 years ago by jdemeyer

I agree with the proposal, but we'll need to see if it passes tests.

comment:5 follow-up: Changed 3 years ago by jhpalmieri

Note that I did not add __hash__ = UniqueRepresentation.__hash__. That can wait until a Python 3 focused ticket, like #24551.

comment:6 Changed 3 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to positive_review

comment:7 in reply to: ↑ 5 Changed 3 years ago by jdemeyer

Replying to jhpalmieri:

Note that I did not add __hash__ = UniqueRepresentation.__hash__.

There shouldn't be a reason for that.

comment:8 Changed 3 years ago by vbraun

  • Branch changed from u/jhpalmieri/abelian_gp_eq to 6c25916cdca7fbfc6f7d89b3bc0c5d04ebea7fdd
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.