Opened 5 years ago

Closed 5 years ago

#21158 closed enhancement (fixed)

Decouple PARI from coercion model

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-7.3
Component: interfaces Keywords:
Cc: defeo 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 5 years ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/21158

comment:2 Changed 5 years ago by jdemeyer

  • Commit set to 9b4a08062f9cc452405e2eebf349d95e98c82c55
  • Dependencies changed from #21126, #20686, #21139, #21140, #21152, #21153, #21154, #20767 to #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 5 years ago by git

  • Commit changed from 9b4a08062f9cc452405e2eebf349d95e98c82c55 to 71f31a494aa21db6651797f582dd4aab2a908f40

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 5 years ago by jdemeyer

  • Status changed from new to needs_review

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

  • Reviewers set to Luca De Feo
  • Status changed from needs_review to 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 in reply to: ↑ 5 Changed 5 years ago by jdemeyer

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 5 years ago by defeo

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 5 years ago by git

  • Commit changed from 71f31a494aa21db6651797f582dd4aab2a908f40 to 91c436752ba82d512efdf67e2890d5ecbe0b7234
  • Status changed from positive_review to needs_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 5 years ago by defeo

  • Status changed from needs_review to positive_review

comment:10 Changed 5 years ago by jdemeyer

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 5 years ago by vbraun

  • Branch changed from u/jdemeyer/ticket/21158 to 91c436752ba82d512efdf67e2890d5ecbe0b7234
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.