Opened 3 months ago

Closed 5 weeks ago

Last modified 9 days ago

#26195 closed enhancement (fixed)

Tate algebras

Reported by: caruso Owned by:
Priority: major Milestone: sage-8.5
Component: padics Keywords:
Cc: TristanVaccon, gh-ThibautVerron, caruso, roed Merged in:
Authors: Xavier Caruso, Thibaut Verron Reviewers: David Roe
Report Upstream: N/A Work issues:
Branch: 4982c7e (Commits) Commit:
Dependencies: #26479, #26575 Stopgaps:

Description (last modified by caruso)

This ticket implements Tate algebras over complete discrete valuation rings/fields, together with Gröbner bases for ideals in these algebras.

See the documentation of sage.rings.tate_algebra for details.

Change History (90)

comment:1 Changed 3 months ago by caruso

  • Branch set to u/caruso/tate_algebras

comment:2 Changed 3 months ago by gh-ThibautVerron

  • Branch changed from u/caruso/tate_algebras to u/gh-ThibautVerron/TateAlgebras
  • Commit set to b8a6ac3ef7ac5f8608e1ff30beb59ced9b85c357

comment:3 Changed 3 months ago by caruso

  • Branch changed from u/gh-ThibautVerron/TateAlgebras to u/caruso/TateAlgebras

comment:4 Changed 3 months ago by git

  • Commit changed from b8a6ac3ef7ac5f8608e1ff30beb59ced9b85c357 to 9ca2244a01a165e24431e9d58acf5054484d0cb3

Branch pushed to git repo; I updated commit sha1. New commits:

9ca2244First working version

comment:5 Changed 3 months ago by git

  • Commit changed from 9ca2244a01a165e24431e9d58acf5054484d0cb3 to 05cf0414d5afae999fdc1f8000075571d9accd52

Branch pushed to git repo; I updated commit sha1. New commits:

05cf041Restriction + small fix in inverse_of_unit

comment:6 Changed 3 months ago by git

  • Commit changed from 05cf0414d5afae999fdc1f8000075571d9accd52 to 5d0e354ec45b362282840aa32d2b4cbd0856285f

Branch pushed to git repo; I updated commit sha1. New commits:

5d0e354Fix bugs in quo_rem

comment:7 Changed 3 months ago by caruso

  • Cc TristanVaccon added; vaccon removed

comment:8 Changed 3 months ago by gh-ThibautVerron

  • Branch changed from u/caruso/TateAlgebras to u/gh-ThibautVerron/tate_algebras
  • Commit changed from 5d0e354ec45b362282840aa32d2b4cbd0856285f to 5abdb64bf364a1459e1b8e771219274fe1e35172

New commits:

fa4e37aFirst docstring
5abdb64Merge branch 'u/caruso/TateAlgebras' of git://trac.sagemath.org/sage into t/26195/TateAlgebras

comment:9 Changed 3 months ago by caruso

  • Branch changed from u/gh-ThibautVerron/tate_algebras to u/caruso/tate_algebras

comment:10 Changed 3 months ago by gh-ThibautVerron

  • Branch changed from u/caruso/tate_algebras to u/gh-ThibautVerron/tate_algebras

comment:11 Changed 3 months ago by git

  • Commit changed from 5abdb64bf364a1459e1b8e771219274fe1e35172 to 78003937e854ba0699ec507d41cd41f7e81d2af8

Branch pushed to git repo; I updated commit sha1. New commits:

7800393Revert changes to .dir-locals.el

comment:12 Changed 3 months ago by git

  • Commit changed from 78003937e854ba0699ec507d41cd41f7e81d2af8 to 2383ba0acfb017721ec55ac44d790f0a1aba81f9

Branch pushed to git repo; I updated commit sha1. New commits:

2383ba0Fixed tests

comment:13 Changed 3 months ago by caruso

  • Cc caruso added

comment:14 Changed 3 months ago by caruso

  • Branch changed from u/gh-ThibautVerron/tate_algebras to u/caruso/tate_algebras

comment:15 Changed 3 months ago by git

  • Commit changed from 2383ba0acfb017721ec55ac44d790f0a1aba81f9 to 8446736d3e9bb14d60b6688f8d258f8a47cb66f6

Branch pushed to git repo; I updated commit sha1. New commits:

8446736Bug fixed in TateAlgebraTerms & more functionalities for ideals

comment:16 Changed 3 months ago by caruso

  • Description modified (diff)

comment:17 Changed 3 months ago by caruso

  • Description modified (diff)

comment:18 Changed 3 months ago by git

  • Commit changed from 8446736d3e9bb14d60b6688f8d258f8a47cb66f6 to fd3bf7b662acc0cebe91f963a8c9057604e69cda

Branch pushed to git repo; I updated commit sha1. New commits:

fd3bf7bSome improvements in Buchberger algorithm

comment:19 Changed 3 months ago by git

  • Commit changed from fd3bf7b662acc0cebe91f963a8c9057604e69cda to bfddf6e4dc8670551fd94ae87d4d330890d1b934

Branch pushed to git repo; I updated commit sha1. New commits:

bfddf6eRewrite in cython

comment:20 Changed 3 months ago by git

  • Commit changed from bfddf6e4dc8670551fd94ae87d4d330890d1b934 to bc7c7637e5d43c612a4137747b1d2d9d2b58d699

Branch pushed to git repo; I updated commit sha1. New commits:

bc7c763Better (?) choice of ordering of S-polynomials

comment:21 Changed 3 months ago by gh-ThibautVerron

  • Branch changed from u/caruso/tate_algebras to u/gh-ThibautVerron/tate_algebras

comment:22 Changed 3 months ago by caruso

  • Branch changed from u/gh-ThibautVerron/tate_algebras to u/caruso/tate_algebras

comment:23 Changed 3 months ago by git

  • Commit changed from bc7c7637e5d43c612a4137747b1d2d9d2b58d699 to b00b8e042c46adde4d0887feb0394293eee50f13

Branch pushed to git repo; I updated commit sha1. New commits:

b00b8e0Use directly the cython class PolyDict

comment:24 Changed 3 months ago by git

  • Commit changed from b00b8e042c46adde4d0887feb0394293eee50f13 to 100cbb727e64017ae940f5c4612a646092c1215a

Branch pushed to git repo; I updated commit sha1. New commits:

100cbb7Fix bug in creation of elements in Tate algebras

comment:25 Changed 3 months ago by gh-ThibautVerron

  • Branch changed from u/caruso/tate_algebras to u/gh-ThibautVerron/tate_algebras

comment:26 Changed 3 months ago by caruso

  • Branch changed from u/gh-ThibautVerron/tate_algebras to u/caruso/tate_algebras

comment:27 Changed 3 months ago by git

  • Commit changed from 100cbb727e64017ae940f5c4612a646092c1215a to 0f6391a172c79ac5620b32f1e323e407ee135016

Branch pushed to git repo; I updated commit sha1. New commits:

0f6391af.residue() now returns a polynomial over the residue field

comment:28 Changed 3 months ago by gh-ThibautVerron

  • Branch changed from u/caruso/tate_algebras to u/gh-ThibautVerron/tate_algebras

comment:29 Changed 3 months ago by git

  • Commit changed from 0f6391a172c79ac5620b32f1e323e407ee135016 to a5b9d368190ce600023393f846a43918cd9533fb

Branch pushed to git repo; I updated commit sha1. New commits:

a5b9d36Partial documentation of Tate algebra term elements

comment:30 Changed 3 months ago by caruso

  • Branch changed from u/gh-ThibautVerron/tate_algebras to u/caruso/tate_algebras

comment:31 Changed 3 months ago by git

  • Commit changed from a5b9d368190ce600023393f846a43918cd9533fb to cfc09d771fab2b7abb49f130913668324969602f

Branch pushed to git repo; I updated commit sha1. New commits:

fc5d3c4Implement rich comparison for terms
cfc09d7Implement monomials() and leading_monomial()

comment:32 Changed 3 months ago by git

  • Commit changed from cfc09d771fab2b7abb49f130913668324969602f to 7c2ac0bc86198f09c364c5151a275f3ad08dcc9c

Branch pushed to git repo; I updated commit sha1. New commits:

7c2ac0bImplement a factory

comment:33 Changed 3 months ago by gh-ThibautVerron

  • Branch changed from u/caruso/tate_algebras to u/gh-ThibautVerron/tate_algebras

comment:34 Changed 3 months ago by git

  • Commit changed from 7c2ac0bc86198f09c364c5151a275f3ad08dcc9c to 2a4044c42980e116e157de9749237f83b08e8058

Branch pushed to git repo; I updated commit sha1. New commits:

2a4044cDocumentation for Tate terms

comment:35 Changed 3 months ago by git

  • Commit changed from 2a4044c42980e116e157de9749237f83b08e8058 to 892c63163df8a1bd0805e6d0e593ac46103ce27d

Branch pushed to git repo; I updated commit sha1. New commits:

439bb96Behavior for rings
0cf12c1Minor changes to docstrings for tests
892c631Fixed some doctests

comment:36 Changed 3 months ago by git

  • Commit changed from 892c63163df8a1bd0805e6d0e593ac46103ce27d to d10fca9e1b2d78e7ce7c9403a272cfd2a93cb31c

Branch pushed to git repo; I updated commit sha1. New commits:

d10fca9Doctest for TateAlgebraElement __init__

comment:37 Changed 3 months ago by caruso

  • Branch changed from u/gh-ThibautVerron/tate_algebras to u/caruso/tate_algebras

comment:38 Changed 3 months ago by git

  • Commit changed from d10fca9e1b2d78e7ce7c9403a272cfd2a93cb31c to 0a49fbb4679a41a7c0a2595bae43ca291516f5a4

Branch pushed to git repo; I updated commit sha1. New commits:

7d6d030Simplify doctests for Tate terms
0a49fbbFix doctest

comment:39 Changed 3 months ago by git

  • Commit changed from 0a49fbb4679a41a7c0a2595bae43ca291516f5a4 to 0576cde54aa26aa7af7d8e2edd68b1c51f4946ba

Branch pushed to git repo; I updated commit sha1. New commits:

0576cdeShorten doctests

comment:40 Changed 3 months ago by git

  • Commit changed from 0576cde54aa26aa7af7d8e2edd68b1c51f4946ba to 53c7ed32c004b8177a5991c7e2d7ea3e160f8119

Branch pushed to git repo; I updated commit sha1. New commits:

53c7ed3Typo

comment:41 Changed 3 months ago by git

  • Commit changed from 53c7ed32c004b8177a5991c7e2d7ea3e160f8119 to fa9b86b913d1526120c1956cb3848798331598eb

Branch pushed to git repo; I updated commit sha1. New commits:

fa9b86bMerge branch 'develop' into t/26195/tate_algebras

comment:42 Changed 3 months ago by gh-ThibautVerron

  • Branch changed from u/caruso/tate_algebras to u/gh-ThibautVerron/tate_algebras

comment:43 Changed 3 months ago by git

  • Commit changed from fa9b86b913d1526120c1956cb3848798331598eb to ef8799f583ef84cadbcec3226b9691ea1b5d3f9b

Branch pushed to git repo; I updated commit sha1. New commits:

ef8799fPartial documentation for the Tate algebra monoid class

comment:44 Changed 3 months ago by git

  • Commit changed from ef8799f583ef84cadbcec3226b9691ea1b5d3f9b to 44ba97030b04cbc846ab44897b6185bbfa90d7ee

Branch pushed to git repo; I updated commit sha1. New commits:

317c782Documentation on Tate terms
44ba970Fixed doctests

comment:45 Changed 3 months ago by caruso

  • Authors set to Xavier Caruso, Thibaut Verron
  • Cc roed added

I cc roed because, David, you might know the answer at the following question.

In this ticket, we have implemented two parents, the first one for the Tate algebra itself and the second one for the monoid of terms of the Tate Algebra. Here is a basic example:

sage: A.<x,y> = TateAlgebra(Zp(2)); A
Tate Algebra in x (val >= 0), y (val >= 0) over 2-adic Field with capped relative precision 20
sage: T = A.monoid_of_terms(); T
Monoid of terms in x (val >= 0), y (val >= 0) over 2-adic Field with capped relative precision 20

We now want to add a method for recovering A (the Tate algebra) from T (the monoid of terms). However a method algebra already exists: it is inherited from sage.categories.sets_cat.Sets and provides a very general construction of an algebra over an arbitrary set. We believe that this general construction is not very relevant in our setting and that T.algebra() should instead output A. So our question is: is it allowed/appropriate to override the method algebra (keeping in mind that it comes from the category machinery)?

In any case, we will create a method tate_algebra but we think that having algebra at the same time (with a different behavior) could be confusing.

Last edited 3 months ago by caruso (previous) (diff)

comment:46 Changed 3 months ago by tscrim

Yes, you are allowed (maybe even encouraged) to override algebra, and in this case, you should because you have a specialized implementation of the algebra.

Also, I want to just point out a common idiom that we've used is to define the monoid of monomials, then use CombinatorialFreeModule and the category of AlgebrasWithBasis to construct the corresponding algebra by implementing product_on_basis. Although this has been known to have some issues with overhead due to the number of indirections (which can be mitigated by directly implementing the multiplication in the element class), and this might not work for what you need.

comment:47 Changed 3 months ago by git

  • Commit changed from 44ba97030b04cbc846ab44897b6185bbfa90d7ee to c559f1487c26ba91644d2fbf83333c66a91c8ac0

Branch pushed to git repo; I updated commit sha1. New commits:

4af77f3Documentation of Buchberger, without examples
cf2afe6Deduplicate coercion + recover parent algebra from monoid of terms
c559f14Documentation of the new function

comment:48 Changed 3 months ago by git

  • Commit changed from c559f1487c26ba91644d2fbf83333c66a91c8ac0 to 298c253bb13e147604e91781f30bf9402fbacae5

Branch pushed to git repo; I updated commit sha1. New commits:

46d8bd1Documentation for ideals
298c253Moved coercion code back in the algebra class

comment:49 Changed 3 months ago by git

  • Commit changed from 298c253bb13e147604e91781f30bf9402fbacae5 to 52b9ad10d6e0b8c8901556090715f9944f594ee5

Branch pushed to git repo; I updated commit sha1. New commits:

52b9ad1Documentation of Buchberger's function

comment:50 Changed 2 months ago by caruso

  • Branch changed from u/gh-ThibautVerron/tate_algebras to u/caruso/tate_algebras

comment:51 Changed 2 months ago by git

  • Commit changed from 52b9ad10d6e0b8c8901556090715f9944f594ee5 to a433e9c22e2a0693a78c851f5b0de615516fd546

Branch pushed to git repo; I updated commit sha1. New commits:

c1acf5cAdd doctest in polydict
af6ddbeAdd licence
66844adAdd tutorial
fc2d6a7Add LaTeX representations
a433e9cAdd evaluation and composition

comment:52 Changed 2 months ago by git

  • Commit changed from a433e9c22e2a0693a78c851f5b0de615516fd546 to 408512dee2f068e87131cf118b59ffb52882bd2e

Branch pushed to git repo; I updated commit sha1. New commits:

408512dNormalize output of groebner_basis

comment:53 Changed 2 months ago by caruso

  • Description modified (diff)
  • Status changed from new to needs_review

comment:54 Changed 2 months ago by git

  • Commit changed from 408512dee2f068e87131cf118b59ffb52882bd2e to 65e7edb02da7421ecbda770e4f437d41ffc33830

Branch pushed to git repo; I updated commit sha1. New commits:

65e7edbAdd doctest to the method dotprod

comment:55 Changed 2 months ago by git

  • Commit changed from 65e7edb02da7421ecbda770e4f437d41ffc33830 to 4966e04a1646b80749c653e9c937be95726d3b72

Branch pushed to git repo; I updated commit sha1. New commits:

4966e04Uncapitalize error messages

comment:56 Changed 2 months ago by git

  • Commit changed from 4966e04a1646b80749c653e9c937be95726d3b72 to 0c8f8019641d90ebb09add1e4c06a64f24840986

Branch pushed to git repo; I updated commit sha1. New commits:

0c8f801Small fixes after patchbot's review

comment:57 Changed 2 months ago by roed

  • Branch changed from u/caruso/tate_algebras to u/roed/tate_algebras

comment:58 Changed 2 months ago by caruso

  • Branch changed from u/roed/tate_algebras to u/caruso/tate_algebras

comment:59 Changed 2 months ago by git

  • Commit changed from 0c8f8019641d90ebb09add1e4c06a64f24840986 to 7c4b4ae4bcda06deac867ae089814a96cc7d1cca

Branch pushed to git repo; I updated commit sha1. New commits:

7c4b4aeAddress David's comments on Zulip

comment:60 Changed 2 months ago by git

  • Commit changed from 7c4b4ae4bcda06deac867ae089814a96cc7d1cca to 30b25af2653550bf07b0bd46bb7d5af1bf74e6aa

Branch pushed to git repo; I updated commit sha1. New commits:

d65ffe0Add log, exp and their friends
30b25afMerge branch 'u/caruso/tate_algebras' of git://trac.sagemath.org/sage into t/26195/tate_algebras

comment:61 Changed 2 months ago by git

  • Commit changed from 30b25af2653550bf07b0bd46bb7d5af1bf74e6aa to 62279371230826c7d4e6c221ce0d00b9620838ff

Branch pushed to git repo; I updated commit sha1. New commits:

6227937Accelerate (?) square roots and nth roots

comment:62 Changed 2 months ago by git

  • Commit changed from 62279371230826c7d4e6c221ce0d00b9620838ff to dcd4c5a005822b6615b41c72c04aa0779e64ac59

Branch pushed to git repo; I updated commit sha1. New commits:

6a1d76aDiscard terms which are indistinguishable from zero in quo_rem
2323508Change "AA" to "Ao" for the ring of integers
dcd4c5aAdd definition of S-polynomial

comment:63 Changed 2 months ago by git

  • Commit changed from dcd4c5a005822b6615b41c72c04aa0779e64ac59 to 5f64c3aa69a013f880c2565a4a20a069aba643a7

Branch pushed to git repo; I updated commit sha1. New commits:

5f64c3aSmall improvement

comment:64 Changed 8 weeks ago by git

  • Commit changed from 5f64c3aa69a013f880c2565a4a20a069aba643a7 to a064d262d34d375d332597b8d56e67a967d4ccf0

Branch pushed to git repo; I updated commit sha1. New commits:

a064d26Add a special case in __pow__ when the exponent is a rational number

comment:65 Changed 8 weeks ago by git

  • Commit changed from a064d262d34d375d332597b8d56e67a967d4ccf0 to 679a4267af30aac8c4e38911345e90cb0285092d

Branch pushed to git repo; I updated commit sha1. New commits:

d5cd2bdAdd documentation to lift_to_precision()
679a426Fix pyflakes

comment:66 Changed 8 weeks ago by git

  • Commit changed from 679a4267af30aac8c4e38911345e90cb0285092d to 1ada3fc97636a19a6824a0976777dbd033ee4a25

Branch pushed to git repo; I updated commit sha1. New commits:

8419d2cfix latex printing
a351051let latex_variable_name handle numeric names
8b18164Add doctest
1ada3fcMerge branch 'padic_printer_latex' into t/26195/tate_algebras

comment:67 Changed 8 weeks ago by caruso

  • Dependencies set to 26479

comment:68 Changed 8 weeks ago by caruso

  • Dependencies changed from 26479 to #26479

comment:69 Changed 8 weeks ago by git

  • Commit changed from 1ada3fc97636a19a6824a0976777dbd033ee4a25 to e48860caec0f9095b37229f01051350fc09a78a6

Branch pushed to git repo; I updated commit sha1. New commits:

5ca2bd0Replace back \Bold{Z} by \ZZ, etc. in the documentation
83b48e3Replace \mathbb{Z}, \mathbb{Q} by \ZZ, \QQ respectively
008d7fcReplace \mathbb{f} by \Bold{F}
e48860cMerge branch 'padic_printer_latex' into t/26195/tate_algebras

comment:70 Changed 8 weeks ago by caruso

I wanted to point out a couple of points:

  • for the methods log and exp, I essentially copy the code in sage.rings.padics.padic_element_generic; so I wonder if it would be a good idea to factorize this code. One solution would be to split the current class pAdicElementGeneric into two parts (e.g pAdicElementGeneric and pAdicNumberGeneric), the second one inheriting from the first one, and let Tate series derive from the most general one.
  • in the method lift_to_precision, I defined a nested function lift_without_error but I think it would be better to add a keyword argument no_error (or something like that) to lift_to_precision (for p-adic number); what's your opinion?

comment:71 Changed 8 weeks ago by git

  • Commit changed from e48860caec0f9095b37229f01051350fc09a78a6 to 1ac31a95865b61d494b9f42b924d2c112d5c204d

Branch pushed to git repo; I updated commit sha1. New commits:

e83713cRevert "Replace \mathbb{f} by \Bold{F}"
2e41efdRevert "Replace \mathbb{Z}, \mathbb{Q} by \ZZ, \QQ respectively"
9e85f91Small fixes
1ac31a9Merge branch 'padic_printer_latex' into t/26195/tate_algebras

comment:72 Changed 8 weeks ago by git

  • Commit changed from 1ac31a95865b61d494b9f42b924d2c112d5c204d to adec9377c4d7b6e25ea2dcb4d7afe083a2ba2f8e

Branch pushed to git repo; I updated commit sha1. New commits:

fb3bbaaFix doctests
adec937Merge branch 'padic_printer_latex' into t/26195/tate_algebras

comment:73 Changed 7 weeks ago by roed

  • Branch changed from u/caruso/tate_algebras to u/roed/tate_algebras

comment:74 Changed 7 weeks ago by caruso

  • Branch changed from u/roed/tate_algebras to u/caruso/tate_algebras
  • Commit changed from adec9377c4d7b6e25ea2dcb4d7afe083a2ba2f8e to 9bae27371b3ceb1b5ffc258d2de6cbdad7df3075

comment:75 Changed 6 weeks ago by roed

  • Dependencies changed from #26479 to #26479, #26575

comment:76 Changed 6 weeks ago by git

  • Commit changed from 9bae27371b3ceb1b5ffc258d2de6cbdad7df3075 to d8c17c992b07579524b454daf29f3b5cf8627f81

Branch pushed to git repo; I updated commit sha1. New commits:

724bc71Allow ... at beginning of doctest output
0fa318fNo longer substitute for ... inside ....:, fix doctests
f02ffa1Merge branch 'develop' into t/26575/ellipses
fc8df57Replace back <ELLIPSIS_TAG> by ... in exc_msg
bad89d2Merge branch 't/26575/ellipses' into t/26195/tate_algebras
d8c17c9Remove parentheses and fix bigoh notation in doctests

comment:77 Changed 6 weeks ago by git

  • Commit changed from d8c17c992b07579524b454daf29f3b5cf8627f81 to 4982c7e44bb87706362b18c981b1f4646203d5b5

Branch pushed to git repo; I updated commit sha1. New commits:

4151b7eFix doctest
4982c7eMerge branch 't/26575/ellipses' into t/26195/tate_algebras

comment:78 Changed 6 weeks ago by roed

  • Reviewers set to David Roe
  • Status changed from needs_review to positive_review

Looks good!

comment:79 Changed 5 weeks ago by chapoton

  • Milestone changed from sage-8.4 to sage-8.5

comment:80 Changed 5 weeks ago by vbraun

  • Branch changed from u/caruso/tate_algebras to 4982c7e44bb87706362b18c981b1f4646203d5b5
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:81 Changed 4 weeks ago by chapoton

  • Commit 4982c7e44bb87706362b18c981b1f4646203d5b5 deleted

Tate algebras elements are now among the 10 files having the most failures with python3. See #26212.

EDIT: And this is also bad:

sage -t --long src/sage/rings/tate_algebra_ideal.pyx  # 29 doctests failed

One example of failure: bad use of range:

File "sage/rings/tate_algebra_ideal.pyx", line 83, in sage.rings.tate_algebra_ideal._groebner_basis_buchberger (build/cythonized/sage/rings/tate_algebra_ideal.c:3488)
        indices = range(l)
    TypeError: Expected list, got range

Other failure: change dict while iterating over this dict:

      File "sage/rings/tate_algebra_element.pyx", line 1151, in sage.rings.tate_algebra_element.TateAlgebraElement._normalize (build/cythonized/sage/rings/tate_algebra_element.c:12421)
        for (e,c) in self._poly.__repn.items():
    RuntimeError: dictionary changed size during iteration
Last edited 4 weeks ago by chapoton (previous) (diff)

comment:82 Changed 4 weeks ago by chapoton

python3 problems are fixed in #26692, please review

comment:83 Changed 4 weeks ago by roed

Sorry about that! I didn't see any py3 failures on the patchbot, so I thought it was okay. I've got a py3 build, so I'll try to check big tickets there from now on.

comment:84 Changed 12 days ago by jdemeyer

This should be fixed:

[288/306] Cythonizing sage/rings/tate_algebra_element.pyx
warning: sage/rings/tate_algebra_element.pyx:301:10: Compatible but non-identical C method '_mul_' not redeclared in definition part of extension type 'TateAlgebraTerm'.  This may cause incorrect vtables to be generated.
warning: sage/structure/element.pxd:188:15: Previous declaration is here

comment:85 Changed 12 days ago by jdemeyer

Essentially, you're missing a declaration for cpdef _mul_ in tate_algebra_element.pxd

comment:86 Changed 12 days ago by jdemeyer

There are also some thing I consider bad style, for example

    @coerce_binop
    def __floordiv__(self, other):

Why not use _floordiv_ from the coercion model?

There is also the inefficient

from sage.structure.richcmp import op_EQ, op_NE, op_LT, op_GT, op_LE, op_GE

which should be

from cpython.object cimport Py_LT, Py_LE, Py_EQ, Py_NE, Py_GT, Py_GE

comment:87 follow-up: Changed 12 days ago by jdemeyer

And isn't _pushout_family() really just coercion_model.common_parent()?

comment:88 Changed 12 days ago by jdemeyer

This is reinventing sage.structure.richcmp.rich_to_bool:

        if op == op_LT:
            return c < 0
        if op == op_LE:
            return c <= 0
        if op == op_GT:
            return c > 0
        if op == op_GE:
            return c >= 0

comment:89 in reply to: ↑ 87 Changed 9 days ago by caruso

Thanks Jeroen for your useful comments!

I addressed all of them except this one:

And isn't _pushout_family() really just coercion_model.common_parent()?

because I'm not sure that _pushout_family() and coercion_model.common_parent() do the same job. At least if I replace the code of _pushout_family() by:

    elements = [initial] + list(elements)
    return coercion_model.common_parent(*elements)

I get many doctest failures.

comment:90 Changed 9 days ago by caruso

cf ticket #26807

Note: See TracTickets for help on using tickets.