Skip to content

Commit

Permalink
gh-37721: Renamed "ring" argument of matrix contructor to "base_ring"
Browse files Browse the repository at this point in the history
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes #12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes #12345". -->

The matrix constructor used to accept ``ring`` as argument to specify a
the base ring of a matrix.
It now takes ``base_ring`` instead. ``ring`` is still accepted but
deprecated (I added a deprecation in the code).
This is for unification (``base_ring`` is what users expect from their
experience with other classes).
This solves @mkoeppe's issue #33380.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [x] I have created tests covering the changes.
- [x] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - #12345: short description why this is a dependency -->
<!-- - #34567: ... -->
    
URL: #37721
Reported by: SandwichGouda
Reviewer(s): Matthias Köppe
  • Loading branch information
Release Manager committed Apr 7, 2024
2 parents bcbb58f + 8378fe6 commit 01333df
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 13 deletions.
10 changes: 8 additions & 2 deletions src/sage/matrix/args.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ from sage.structure.element cimport Element, RingElement, Vector
from sage.arith.long cimport pyobject_to_long
from sage.misc.misc_c import sized_iter
from sage.categories import monoids
from sage.misc.superseded import deprecation_cython


try:
Expand Down Expand Up @@ -311,7 +312,7 @@ cdef class MatrixArgs:
self.sparse = -1
self.kwds = {}

def __init__(self, *args, ring=None, nrows=None, ncols=None, entries=None, sparse=None, space=None, **kwds):
def __init__(self, *args, base_ring=None, nrows=None, ncols=None, entries=None, sparse=None, space=None, **kwds):
"""
Parse arguments for creating a new matrix.
Expand Down Expand Up @@ -340,7 +341,12 @@ cdef class MatrixArgs:
sage: MatrixArgs(3, ncols=1).finalized()
<MatrixArgs for Full MatrixSpace of 1 by 1 dense matrices over Integer Ring; typ=SCALAR; entries=3>
"""
self.base = ring
if "ring" in kwds.keys():
deprecation_cython(issue_number=33380, message="ring is deprecated (keyword will be removed in the future). Use base_ring instead", stacklevel=3)
self.base = kwds.pop("ring")
else:
self.base = base_ring

if nrows is not None:
self.set_nrows(pyobject_to_long(nrows))
if ncols is not None:
Expand Down
12 changes: 6 additions & 6 deletions src/sage/matrix/constructor.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def matrix(*args, **kwds):
Positional and keyword arguments:
- ``ring`` -- parent of the entries of the matrix (despite the
- ``base_ring`` -- parent of the entries of the matrix (despite the
name, this is not a priori required to be a ring). By default,
determine this from the given entries, falling back to ``ZZ`` if
no entries are given.
Expand All @@ -78,7 +78,7 @@ def matrix(*args, **kwds):
defaults to ``False``.
- ``space`` -- matrix space which will be the parent of the output
matrix. This determines ``ring``, ``nrows``, ``ncols`` and
matrix. This determines ``base_ring``, ``nrows``, ``ncols`` and
``sparse``.
- ``immutable`` -- (boolean) make the matrix immutable. By default,
Expand Down Expand Up @@ -254,13 +254,13 @@ def matrix(*args, **kwds):
sage: m = matrix(QQ); m; m.parent()
[]
Full MatrixSpace of 0 by 0 dense matrices over Rational Field
sage: m = matrix(ring=QQ); m; m.parent()
sage: m = matrix(base_ring=QQ); m; m.parent()
[]
Full MatrixSpace of 0 by 0 dense matrices over Rational Field
sage: m = matrix(0); m; m.parent()
[]
Full MatrixSpace of 0 by 0 dense matrices over Integer Ring
sage: m = matrix(0, 0, ring=QQ); m; m.parent()
sage: m = matrix(0, 0, base_ring=QQ); m; m.parent()
[]
Full MatrixSpace of 0 by 0 dense matrices over Rational Field
sage: m = matrix([]); m; m.parent()
Expand Down Expand Up @@ -612,9 +612,9 @@ def matrix(*args, **kwds):
sage: parent(M) is S
True
A redundant ``ring`` argument::
A redundant ``base_ring`` argument::
sage: matrix(ZZ, 3, 3, ring=ZZ)
sage: matrix(ZZ, 3, 3, base_ring=ZZ)
Traceback (most recent call last):
...
TypeError: too many arguments in matrix constructor
Expand Down
10 changes: 5 additions & 5 deletions src/sage/matrix/special.py
Original file line number Diff line number Diff line change
Expand Up @@ -3454,7 +3454,7 @@ def ith_to_zero_rotation_matrix(v, i, ring=None):
bb = b / norm
entries = {(k, k): 1 for k in range(dim)}
entries.update({(j, j): aa, (j, i): bb, (i, j): -bb, (i, i): aa})
return matrix(entries, nrows=dim, ring=ring)
return matrix(entries, nrows=dim, base_ring=ring)


@matrix_method
Expand Down Expand Up @@ -3488,7 +3488,7 @@ def hilbert(dim, ring=QQ):
"""
def entries(i, j):
return ZZ.one() / (i + j + 1)
return matrix(entries, nrows=dim, ncols=dim, ring=ring)
return matrix(entries, nrows=dim, ncols=dim, base_ring=ring)


@matrix_method
Expand Down Expand Up @@ -3522,7 +3522,7 @@ def vandermonde(v, ring=None):
"""
def entries(i, j):
return v[i]**j
return matrix(entries, nrows=len(v), ncols=len(v), ring=ring)
return matrix(entries, nrows=len(v), ncols=len(v), base_ring=ring)


@matrix_method
Expand Down Expand Up @@ -3568,7 +3568,7 @@ def toeplitz(c, r, ring=None):
"""
def entries(i, j):
return c[i - j] if i >= j else r[j - i - 1]
return matrix(entries, nrows=len(c), ncols=len(r)+1, ring=ring)
return matrix(entries, nrows=len(c), ncols=len(r)+1, base_ring=ring)


@matrix_method
Expand Down Expand Up @@ -3638,4 +3638,4 @@ def hankel(c, r=None, ring=None):

def entries(i):
return c[i] if i < m else r[i - m]
return matrix(lambda i, j: entries(i + j), nrows=m, ncols=n + 1, ring=ring)
return matrix(lambda i, j: entries(i + j), nrows=m, ncols=n + 1, base_ring=ring)

0 comments on commit 01333df

Please sign in to comment.