Opened 6 years ago

Closed 6 years ago

#21158 closed enhancement (fixed)

Decouple PARI from coercion model

Reported by: Jeroen Demeyer Owned by:
Priority: major Milestone: sage-7.3
Component: interfaces Keywords:
Cc: Luca De Feo Merged in:
Authors: Jeroen Demeyer Reviewers: Luca De Feo
Report Upstream: N/A Work issues:
Branch: 91c4367 (Commits, GitHub, GitLab) Commit: 91c436752ba82d512efdf67e2890d5ecbe0b7234
Dependencies: #21126, #20686, #21139, #21140, #21152, #21153, #21154, #20767, #21163 Stopgaps:

Status badges

Description


Change History (11)

comment:1 Changed 6 years ago by Jeroen Demeyer

Branch: u/jdemeyer/ticket/21158

comment:2 Changed 6 years ago by Jeroen Demeyer

Commit: 9b4a08062f9cc452405e2eebf349d95e98c82c55
Dependencies: #21126, #20686, #21139, #21140, #21152, #21153, #21154, #20767#21126, #20686, #21139, #21140, #21152, #21153, #21154, #20767, #21163

This branch already mostly works. The only remaining issue:

sage: 2 == pari(2)
False

This will be fixed in #21163.


Last 10 new commits:

fbcda3bMerge branch 't/21139/implement_negation_for_modular_forms' into t/20767/move_coercion_to_element
69d18c7Remove redundant _lmul_ and _rmul_ methods
2f8e1c9Merge commit '69d18c75875c8a9f770e9ef493d6e721854f4448' into t/20767/move_coercion_to_element
12d9d32Implement unary operations in interfaces
aae48bdMerge branch 'ticket/21152' into t/20767/move_coercion_to_element
0a44b08Remove one doctest for __mul__
54b0fcdVarious minor fixes for the coercion model
2405ddeMerge commit '0a44b082f08c742fbe564c5afdd2f7309fabbb52'; commit '54b0fcdaf2fd69cd64978da526af6774b57a59a8' into t/20767/move_coercion_to_element
e1d2ba4Move arithmetic using coercion model to Element
9b4a080Decouple PARI from the Sage coercion model

comment:3 Changed 6 years ago by git

Commit: 9b4a08062f9cc452405e2eebf349d95e98c82c5571f31a494aa21db6651797f582dd4aab2a908f40

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

b3ea04fTry reversed operation in richcmp if coercion fails
efc4fd0Merge branch 't/21163/in_richcmp__fall_back_to_reversed_operation_if_coercion_fails' into t/21158/ticket/21158
71f31a4Decouple PARI from the Sage coercion model

comment:4 Changed 6 years ago by Jeroen Demeyer

Status: newneeds_review

comment:5 Changed 6 years ago by Luca De Feo

Reviewers: Luca De Feo
Status: needs_reviewpositive_review

Looks good to me. Thanks. I'll rebase my work off this ticket, that's going to help a lot!

Do you need reviewers for the dependencies? I feel I'm not the most competent for those, but I can give a shot.

comment:6 in reply to:  5 Changed 6 years ago by Jeroen Demeyer

Replying to defeo:

Do you need reviewers for the dependencies? I feel I'm not the most competent for those, but I can give a shot.

I hope that Nicolas Thiéry will deal with #20686. But #20767 is the big one, I'm writing to sage-devel about it.

comment:7 Changed 6 years ago by Luca De Feo

Just stumbled upon this line while cleaning the PARI interface: https://git.sagemath.org/sage.git/tree/src/sage/symbolic/ring.pyx?id=824f7ead9d7668e322e8a02670c60d39467b2610#n199

Might have to be addressed by this ticket?

comment:8 Changed 6 years ago by git

Commit: 71f31a494aa21db6651797f582dd4aab2a908f4091c436752ba82d512efdf67e2890d5ecbe0b7234
Status: positive_reviewneeds_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

91c4367Remove useless "return False" in SR._coerce_map_from_()

comment:9 Changed 6 years ago by Luca De Feo

Status: needs_reviewpositive_review

comment:10 Changed 6 years ago by Jeroen Demeyer

About dependencies: #21126 is a pretty trivial Cython patch. I don't think that upstream could have anything against it, but they sometimes take time to look at pull requests...

comment:11 Changed 6 years ago by Volker Braun

Branch: u/jdemeyer/ticket/2115891c436752ba82d512efdf67e2890d5ecbe0b7234
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.