sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.33k stars 453 forks source link

Make is_integral_domain() have the same signature #34372

Closed tscrim closed 2 years ago

tscrim commented 2 years ago

This is not fully consistent within Sage:

travis@tscrim:~/sage-build$ grep -R "def is_integral_domain" src/sage/*
src/sage/algebras/steenrod/steenrod_algebra.py:    def is_integral_domain(self, proof=True):
src/sage/algebras/quatalg/quaternion_algebra.py:    def is_integral_domain(self, proof=True) -> bool:
src/sage/categories/integral_domains.py:        def is_integral_domain(self):
src/sage/categories/group_algebras.py:        def is_integral_domain(self, proof=True):
src/sage/combinat/sf/sfa.py:        def is_integral_domain(self, proof=True):
src/sage/manifolds/chart_func.py:    def is_integral_domain(self, proof=True):
src/sage/rings/quotient_ring.py:    def is_integral_domain(self, proof=True):
src/sage/rings/ring.pyx:    def is_integral_domain(self, proof = True):
src/sage/rings/ring.pyx:    def is_integral_domain(self, proof = True):
src/sage/rings/finite_rings/integer_mod_ring.py:    def is_integral_domain(self, proof=None):
src/sage/rings/multi_power_series_ring.py:    def is_integral_domain(self, proof=False):
src/sage/rings/polynomial/laurent_polynomial_ring.py:    def is_integral_domain(self, proof=True):
src/sage/rings/polynomial/multi_polynomial_ring_base.c: *     def is_integral_domain(self, proof=True):
src/sage/rings/polynomial/multi_polynomial_ring_base.c: *     def is_integral_domain(self, proof=True):             # <<<<<<<<<<<<<<
src/sage/rings/polynomial/multi_polynomial_ring_base.c: *     def is_integral_domain(self, proof=True):             # <<<<<<<<<<<<<<
src/sage/rings/polynomial/polynomial_quotient_ring.py:    def is_integral_domain(self, proof = True):
src/sage/rings/polynomial/infinite_polynomial_ring.py:    def is_integral_domain(self, *args, **kwds):
src/sage/rings/polynomial/polynomial_ring.py:    def is_integral_domain(self, proof = True):
src/sage/rings/polynomial/multi_polynomial_ring_base.pyx:    def is_integral_domain(self, proof=True):
src/sage/rings/polynomial/multi_polynomial_ring.py:    def is_integral_domain(self, proof=True):
src/sage/rings/tate_algebra.py:    def is_integral_domain(self):
src/sage/rings/ring.c: *     def is_integral_domain(self, proof = True):
src/sage/rings/ring.c: *     def is_integral_domain(self, proof = True):             # <<<<<<<<<<<<<<
src/sage/rings/ring.c: *     def is_integral_domain(self, proof = True):             # <<<<<<<<<<<<<<
src/sage/rings/ring.c: *     def is_integral_domain(self, proof = True):
src/sage/rings/ring.c: *     def is_integral_domain(self, proof = True):             # <<<<<<<<<<<<<<
src/sage/rings/ring.c: *     def is_integral_domain(self, proof = True):             # <<<<<<<<<<<<<<

This causes failures for the tests that also pass a proof argument. For example:

sage: A.<x,y> = TateAlgebra(Zp(3))
sage: R.<a,b> = PolynomialRing(A)
sage: R.is_integral_domain()
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Input In [3], in <cell line: 1>()
----> 1 R.is_integral_domain()

File ~/sage/src/sage/rings/polynomial/multi_polynomial_ring_base.pyx:108, in sage.rings.polynomial.multi_polynomial_ring_base.MPolynomialRing_base.is_integral_domain()
    106         False
    107     """
--> 108     return self.base_ring().is_integral_domain(proof)
    109 
    110 def is_noetherian(self):

TypeError: TateAlgebra_generic.is_integral_domain() takes 1 positional argument but 2 were given

CC: @mantepse @fchapoton

Component: algebra

Author: Travis Scrimshaw

Branch/Commit: ae8cfa3

Reviewer: Frédéric Chapoton

Issue created by migration from https://trac.sagemath.org/ticket/34372

tscrim commented 2 years ago

Description changed:

--- 
+++ 
@@ -30,4 +30,23 @@
 src/sage/rings/ring.c: *     def is_integral_domain(self, proof = True):             # <<<<<<<<<<<<<<
 src/sage/rings/ring.c: *     def is_integral_domain(self, proof = True):             # <<<<<<<<<<<<<<

-This causes failures for the tests that also pass a proof argument (such as polynomial rings). +This causes failures for the tests that also pass a proof argument. For example: + +``` +sage: A.<x,y> = TateAlgebra(Zp(3)) +sage: R.<a,b> = PolynomialRing(A) +sage: R.is_integral_domain() +--------------------------------------------------------------------------- +TypeError Traceback (most recent call last) +Input In [3], in <cell line: 1>() +----> 1 R.is_integral_domain() + +File ~/sage/src/sage/rings/polynomial/multi_polynomial_ring_base.pyx:108, in sage.rings.polynomial.multi_polynomial_ring_base.MPolynomialRing_base.is_integral_domain()

tscrim commented 2 years ago

Branch: u/tscrim/is_integral_domain_signatures-34372

tscrim commented 2 years ago
comment:2

Fairly straightforward ticket.

I noticed it while trying to make a multivariate lazy Taylor series ring over a lazy Laurent series ring.


New commits:

ae8cfa3Make is_integral_domain() always take a proof keyword arugment.
tscrim commented 2 years ago

Commit: ae8cfa3

fchapoton commented 2 years ago
comment:3

ok, let it be

fchapoton commented 2 years ago

Reviewer: Frédéric Chapoton

tscrim commented 2 years ago
comment:4

Merci.

vbraun commented 2 years ago

Changed branch from u/tscrim/is_integral_domain_signatures-34372 to ae8cfa3