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:  sage7.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: 
Description
Change History (11)
comment:1 Changed 6 years ago by
Branch:  → u/jdemeyer/ticket/21158 

comment:2 Changed 6 years ago by
Commit:  → 9b4a08062f9cc452405e2eebf349d95e98c82c55 

Dependencies:  #21126, #20686, #21139, #21140, #21152, #21153, #21154, #20767 → #21126, #20686, #21139, #21140, #21152, #21153, #21154, #20767, #21163 
comment:3 Changed 6 years ago by
Commit:  9b4a08062f9cc452405e2eebf349d95e98c82c55 → 71f31a494aa21db6651797f582dd4aab2a908f40 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
b3ea04f  Try reversed operation in richcmp if coercion fails

efc4fd0  Merge branch 't/21163/in_richcmp__fall_back_to_reversed_operation_if_coercion_fails' into t/21158/ticket/21158

71f31a4  Decouple PARI from the Sage coercion model

comment:4 Changed 6 years ago by
Status:  new → needs_review 

comment:5 followup: 6 Changed 6 years ago by
Reviewers:  → Luca De Feo 

Status:  needs_review → positive_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 Changed 6 years ago by
comment:7 Changed 6 years ago by
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
Commit:  71f31a494aa21db6651797f582dd4aab2a908f40 → 91c436752ba82d512efdf67e2890d5ecbe0b7234 

Status:  positive_review → needs_review 
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
91c4367  Remove useless "return False" in SR._coerce_map_from_()

comment:9 Changed 6 years ago by
Status:  needs_review → positive_review 

comment:10 Changed 6 years ago by
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
Branch:  u/jdemeyer/ticket/21158 → 91c436752ba82d512efdf67e2890d5ecbe0b7234 

Resolution:  → fixed 
Status:  positive_review → closed 
This branch already mostly works. The only remaining issue:
This will be fixed in #21163.
Last 10 new commits:
Merge branch 't/21139/implement_negation_for_modular_forms' into t/20767/move_coercion_to_element
Remove redundant _lmul_ and _rmul_ methods
Merge commit '69d18c75875c8a9f770e9ef493d6e721854f4448' into t/20767/move_coercion_to_element
Implement unary operations in interfaces
Merge branch 'ticket/21152' into t/20767/move_coercion_to_element
Remove one doctest for __mul__
Various minor fixes for the coercion model
Merge commit '0a44b082f08c742fbe564c5afdd2f7309fabbb52'; commit '54b0fcdaf2fd69cd64978da526af6774b57a59a8' into t/20767/move_coercion_to_element
Move arithmetic using coercion model to Element
Decouple PARI from the Sage coercion model