Opened 3 years ago

Closed 3 years ago

#26279 closed enhancement (fixed)

Improve comparisons of PolyDicts

Reported by: embray Owned by:
Priority: minor Milestone: sage-8.5
Component: refactoring Keywords:
Cc: Merged in:
Authors: Erik Bray Reviewers: Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: 1cfdd4c (Commits, GitHub, GitLab) Commit: 1cfdd4cb6b6fdf80b74fb905339027955cd5d7c2
Dependencies: Stopgaps:

Status badges

Description

The PolyDict class has a __richcmp__ method which just implements the default rich comparison between the underlying dicts of the PolyDicts being compared, which in of itself is not terrible useful, especially given that it's not a meaningful comparison for relational operators.

However there is also an undocumented PolyDict.rich_compare method which i the same as __richcmp__ but takes a key argument (previously required, but I made it optional and renamed it to sortkey) which can be used to provide meaningful ordering between polynomials. This is used currently in Sage in exactly one place. But this ticket shows one other example where it was useful to have. We've also polished up the documentation just a little bit.

Change History (8)

comment:1 Changed 3 years ago by embray

  • Status changed from new to needs_review

comment:2 Changed 3 years ago by chapoton

  • Status changed from needs_review to needs_work

branch is red

comment:3 Changed 3 years ago by embray

  • Milestone changed from sage-8.4 to sage-8.5

comment:4 Changed 3 years ago by git

  • Commit changed from 3edaefa0608c9bd36efa1a0e223da8b5d83edd2c to 5fb5c762830796187163fb00dab42c80803e1950

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​4fd9277some cleanup for PolyDict.rich_compare and __richcmp__
​5fb5c76demonstrate an example where it makes sense to use PolyDict.rich_compare

comment:5 Changed 3 years ago by embray

  • Status changed from needs_work to needs_review

Trivial rebase at first. But I also fixed a test that was failing on Python 2 (the p1 < p2 test) due to the differences in default comparison behavior between Python 2 and 3. Not 100% satisfied with that, and wondering if we should instead explicitly enforce == and != only.

comment:6 Changed 3 years ago by chapoton

  • Branch changed from u/embray/polydict/richcmp to public/26279
  • Commit changed from 5fb5c762830796187163fb00dab42c80803e1950 to 1cfdd4cb6b6fdf80b74fb905339027955cd5d7c2
  • Reviewers set to Frédéric Chapoton
  • Status changed from needs_review to positive_review

I have corrected a typo and changed 2 http to https.

This allows many doctests to pass in Iwahori-Hecke algebras with python3.

New commits:

​ad058d6Merge branch 'u/embray/polydict/richcmp' in 8.5.b2
​1cfdd4ctrac 26279 some http details and one typo

comment:7 Changed 3 years ago by embray

Thanks!

comment:8 Changed 3 years ago by vbraun

  • Branch changed from public/26279 to 1cfdd4cb6b6fdf80b74fb905339027955cd5d7c2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.