Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#22076 closed enhancement (fixed)

cmp -> richcmp for polynomials

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-7.6
Component: python3 Keywords:
Cc: chapoton Merged in:
Authors: Jeroen Demeyer Reviewers: Marc Mezzarobba
Report Upstream: N/A Work issues:
Branch: 65b1205 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by jdemeyer)

For the base class Polynomial and its subclasses, use _richcmp_ instead of _cmp_ to implement comparison.

Note that this does not include multi-variate polynomials.

Change History (11)

comment:1 Changed 5 years ago by jdemeyer

  • Summary changed from cmp -> richcmp for Polynomial to cmp -> richcmp for polynomials

comment:2 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:3 Changed 5 years ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/22076

comment:4 Changed 5 years ago by jdemeyer

  • Commit set to 65b1205ce1fb8564f96d25c347b2e4c2a5026a1f
  • Status changed from new to needs_review

New commits:

65b1205Use richcmp to compare polynomials

comment:5 follow-up: Changed 5 years ago by mmezzarobba

  • Status changed from needs_review to needs_work

I think this is incorrect:

sage: Pol.<x> = RBF[]
sage: Pol(1/3) == Pol(1/3)
False
sage: x/3 == x/3
True

If I remember correctly, there were already bugs of this kind with polynomials, but this particular example shows a regression.

comment:6 Changed 5 years ago by jdemeyer

Right. This is due to

sage: RBF(1/3) == RBF(1/3)
False
sage: RBF(1/3) != RBF(1/3)
False

comment:7 follow-up: Changed 5 years ago by jdemeyer

This shows a deeper issue with the use of richcmp_not_equal (which appears in several places). This specific issue with RBF is therefore not limited to polynomials.

Therefore, I would suggest to accept this ticket as-is and then work on improving every use of richcmp_not_equal on a follow-up ticket. Would that be acceptable?

comment:8 in reply to: ↑ 7 Changed 5 years ago by mmezzarobba

  • Reviewers set to Marc Mezzarobba
  • Status changed from needs_work to positive_review

Replying to jdemeyer:

Therefore, I would suggest to accept this ticket as-is and then work on improving every use of richcmp_not_equal on a follow-up ticket. Would that be acceptable?

To me, yes.

If someone disagrees, please feel free to revert the ticket to needs_work.

comment:9 Changed 5 years ago by chapoton

  • Milestone changed from sage-7.5 to sage-7.6

comment:10 Changed 5 years ago by vbraun

  • Branch changed from u/jdemeyer/ticket/22076 to 65b1205ce1fb8564f96d25c347b2e4c2a5026a1f
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:11 in reply to: ↑ 5 Changed 4 years ago by jdemeyer

  • Commit 65b1205ce1fb8564f96d25c347b2e4c2a5026a1f deleted

Replying to mmezzarobba:

I think this is incorrect:

sage: Pol.<x> = RBF[]
sage: Pol(1/3) == Pol(1/3)
False
sage: x/3 == x/3
True

This is now fixed in #22087.

Note: See TracTickets for help on using tickets.