Opened 5 years ago

Closed 4 years ago

#22344 closed enhancement (fixed)

Parent richcmp: never use id()

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-8.3
Component: coercion Keywords: richcmp, days94
Cc: Merged in:
Authors: Frédéric Chapoton Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: 9cf0503 (Commits, GitHub, GitLab) Commit: 9cf050319e9d5caad61dc8e24096968e3514a60c
Dependencies: Stopgaps:

Status badges

Description

See #22029.

Change History (13)

comment:1 Changed 4 years ago by jdemeyer

  • Keywords richcmp added

comment:2 Changed 4 years ago by chapoton

  • Branch set to public/22344
  • Commit set to 63badaf27c2419dc25a3d06a593cfedaa40cfada

New commits:

​63badafdo not use id in parent comparison

comment:3 Changed 4 years ago by git

  • Commit changed from 63badaf27c2419dc25a3d06a593cfedaa40cfada to 9cf050319e9d5caad61dc8e24096968e3514a60c

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

​9cf0503fix

comment:4 Changed 4 years ago by chapoton

  • Milestone changed from sage-7.6 to sage-8.3
  • Status changed from new to needs_review

well.. Bot seems to be morally green !

comment:5 Changed 4 years ago by jdemeyer

Interesting... so, this _cmp_ method is never used?

comment:6 Changed 4 years ago by tscrim

My guess is that between parents being UniqueRepresentation (or UniqueFactory) and those that are not directly having to implement comparisons, this is no longer used.

Although I am slightly cautious about including this in such a late beta cycle. So I am inclined to wait for 8.4.beta0, but that is not a strong position. Thoughts?

comment:7 Changed 4 years ago by chapoton

  • Authors set to Frédéric Chapoton

comment:8 Changed 4 years ago by chapoton

so do we try this now or wait until 8.4.beta0 ? any other opinion ?

comment:9 Changed 4 years ago by jdemeyer

I'm currently at Sage Days 94 with Travis, I can talk to him.

comment:10 Changed 4 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer

Let me just do one more test and then I'll set this to positive review.

comment:11 Changed 4 years ago by jdemeyer

  • Keywords days94 added

comment:12 Changed 4 years ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:13 Changed 4 years ago by vbraun

  • Branch changed from public/22344 to 9cf050319e9d5caad61dc8e24096968e3514a60c
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.