Opened 4 years ago
Closed 4 years ago
#22907 closed enhancement (fixed)
py3: phase out lexico cmp in real_mpfi
Reported by: | chapoton | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.0 |
Component: | python3 | Keywords: | |
Cc: | tscrim, jdemeyer, aapitzsch, dkrenn, cheuberg, behackl | Merged in: | |
Authors: | Frédéric Chapoton | Reviewers: | Daniel Krenn |
Report Upstream: | N/A | Work issues: | |
Branch: | ea97bfc (Commits, GitHub, GitLab) | Commit: | ea97bfce2d685fffcb7c17ac5967be875385806a |
Dependencies: | #18303 | Stopgaps: |
Description
Currently cmp(a,b) for two real-interval field elements performs a lexicographic comparison. And rich comparison has a different semantic.
We rename _cmp_
to lexico_cmp
to put it outside the comparison framework. This means that cmp
will not work anymore. The documentation is modified accordingly, to warn users not to use cmp
at all on these objects.
Helpful for the major ticket #22297
Change History (16)
comment:1 Changed 4 years ago by
- Branch set to u/chapoton/22907
- Cc tscrim jdemeyer aapitzsch dkrenn cheuberg behackl added
- Commit set to 20cd81e1bee338a47aa5a77906e9747df03d2592
- Status changed from new to needs_review
comment:2 in reply to: ↑ description Changed 4 years ago by
comment:3 follow-up: ↓ 4 Changed 4 years ago by
well, lexico_cmp is useful to check that pickling works. And if somebody did use cmp, it could serve as a replacement
comment:4 in reply to: ↑ 3 ; follow-up: ↓ 5 Changed 4 years ago by
Replying to chapoton:
well, lexico_cmp is useful to check that pickling works. And if somebody did use cmp, it could serve as a replacement
Ok, fine for me.
comment:5 in reply to: ↑ 4 Changed 4 years ago by
comment:6 Changed 4 years ago by
- Reviewers set to Daniel Krenn
comment:7 Changed 4 years ago by
Apart from the question above and modulo a successful run of a patchbot, this patch looks good.
comment:8 Changed 4 years ago by
Hmm, trying to introduce a deprecation seems to uncover some problems with QQbar. Investigating, maybe in relation with #18303
comment:9 follow-up: ↓ 11 Changed 4 years ago by
en experimental branch with deprecation is available as "u/chapoton/experiment-22907"
comment:10 Changed 4 years ago by
- Status changed from needs_review to needs_work
comment:11 in reply to: ↑ 9 Changed 4 years ago by
Replying to chapoton:
en experimental branch with deprecation is available as "u/chapoton/experiment-22907"
This looks fine for me.
comment:12 Changed 4 years ago by
- Branch changed from u/chapoton/22907 to u/chapoton/experiment-22907
- Commit changed from 20cd81e1bee338a47aa5a77906e9747df03d2592 to ea97bfce2d685fffcb7c17ac5967be875385806a
- Dependencies set to #18303
comment:13 Changed 4 years ago by
looks good, bot is morally green
comment:14 Changed 4 years ago by
- Status changed from needs_work to needs_review
back to needs review, please double check
comment:15 Changed 4 years ago by
- Status changed from needs_review to positive_review
comment:16 Changed 4 years ago by
- Branch changed from u/chapoton/experiment-22907 to ea97bfce2d685fffcb7c17ac5967be875385806a
- Resolution set to fixed
- Status changed from positive_review to closed
Replying to chapoton:
Do we need
lexico_cmp
at all or could we simply delete it?