Skip to content

Commit

Permalink
sagemathgh-37924: Deprecate is_FGP_Module, is_FilteredVectorSpace
Browse files Browse the repository at this point in the history
…, `is_FreeQuadraticModule`, `is_FreeModule`, `is_FreeModuleHomspace`, `is_MatrixSpace`, `is_Module`, `is_VectorSpace`, `is_VectorSpaceHomspace`

    
<!-- ^ 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 sagemath#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 sagemath#12345". -->



### 📝 Checklist

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

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37924
Reported by: Matthias Köppe
Reviewer(s):
  • Loading branch information
Release Manager committed May 14, 2024
2 parents d7f6a14 + 72e46f7 commit 7935745
Show file tree
Hide file tree
Showing 23 changed files with 136 additions and 79 deletions.
4 changes: 2 additions & 2 deletions src/sage/geometry/polyhedron/parent.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from sage.structure.parent import Parent
from sage.structure.element import get_coercion_model
from sage.structure.unique_representation import UniqueRepresentation
from sage.modules.free_module import FreeModule, is_FreeModule
from sage.modules.free_module import FreeModule, FreeModule_generic
from sage.misc.cachefunc import cached_method, cached_function
from sage.misc.lazy_import import lazy_import
import sage.rings.abc
Expand Down Expand Up @@ -1009,7 +1009,7 @@ def _get_action_(self, other, op, self_is_left):
from sage.structure.coerce_actions import ActedUponAction
from sage.categories.action import PrecomposedAction

if op is operator.add and is_FreeModule(other):
if op is operator.add and isinstance(other, FreeModule_generic):
base_ring = self._coerce_base_ring(other)
extended_self = self.base_extend(base_ring)
extended_other = other.base_extend(base_ring)
Expand Down
4 changes: 2 additions & 2 deletions src/sage/groups/matrix_gps/finitely_generated.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
from sage.groups.matrix_gps.group_element import is_MatrixGroupElement
from sage.groups.matrix_gps.matrix_group import MatrixGroup_generic
from sage.matrix.constructor import matrix
from sage.matrix.matrix_space import is_MatrixSpace
from sage.matrix.matrix_space import MatrixSpace
from sage.misc.cachefunc import cached_method
from sage.rings.integer_ring import ZZ
from sage.structure.element import is_Matrix
Expand Down Expand Up @@ -130,7 +130,7 @@ def normalize_square_matrices(matrices):
raise ValueError('not all matrices have the same size')
gens = Sequence(gens, immutable=True)
MS = gens.universe()
if not is_MatrixSpace(MS):
if not isinstance(MS, MatrixSpace):
raise TypeError('all generators must be matrices')
if MS.nrows() != MS.ncols():
raise ValueError('matrices must be square')
Expand Down
4 changes: 2 additions & 2 deletions src/sage/groups/matrix_gps/isometries.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,9 +227,9 @@ def _get_action_(self, S, op, self_on_left):
return GroupActionOnSubmodule(self, S)
if S is self._invariant_quotient_module:
return GroupActionOnQuotientModule(self, S)
from sage.modules.fg_pid.fgp_module import is_FGP_Module
from sage.modules.fg_pid.fgp_module import FGP_Module_class
T = self._invariant_quotient_module
if is_FGP_Module(S):
if isinstance(S, FGP_Module_class):
if S.is_submodule(T):
V = S.V()
if all(V == V * f.matrix() for f in self.gens()):
Expand Down
4 changes: 2 additions & 2 deletions src/sage/manifolds/subsets/pullback.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from sage.categories.sets_cat import Sets, EmptySetError
from sage.categories.metric_spaces import MetricSpaces
from sage.misc.lazy_import import lazy_import
from sage.modules.free_module import is_FreeModule
from sage.modules.free_module import FreeModule_generic
from sage.rings.infinity import infinity, minus_infinity
from sage.rings.integer_ring import ZZ
from sage.rings.rational_field import QQ
Expand Down Expand Up @@ -829,7 +829,7 @@ def is_closed(self):
# Regardless of their base_ring, we treat polyhedra as closed
# convex subsets of R^n
return True
elif is_FreeModule(self._codomain_subset) and self._codomain_subset.rank() != infinity:
elif isinstance(self._codomain_subset, FreeModule_generic) and self._codomain_subset.rank() != infinity:
if self._codomain_subset.base_ring() in MetricSpaces().Complete():
# Closed topological vector subspace
return True
Expand Down
12 changes: 6 additions & 6 deletions src/sage/matrix/action.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ AUTHOR:

import operator

from sage.matrix.matrix_space import MatrixSpace, is_MatrixSpace
from sage.modules.free_module import FreeModule, is_FreeModule
from sage.matrix.matrix_space import MatrixSpace
from sage.modules.free_module import FreeModule, FreeModule_generic
from sage.structure.coerce cimport coercion_model
from sage.categories.homset import Hom, End

Expand Down Expand Up @@ -100,7 +100,7 @@ cdef class MatrixMulAction(Action):
over Univariate Polynomial Ring in x over Rational Field
"""
def __init__(self, G, S, is_left):
if not is_MatrixSpace(G):
if not isinstance(G, MatrixSpace):
raise TypeError("Not a matrix space: %s" % G)
if isinstance(S, SchemeHomset_generic):
if G.base_ring() is not S.domain().base_ring():
Expand Down Expand Up @@ -160,7 +160,7 @@ cdef class MatrixMatrixAction(MatrixMulAction):
example is good practice.
"""
def __init__(self, G, S):
if not is_MatrixSpace(S):
if not isinstance(S, MatrixSpace):
raise TypeError("Not a matrix space: %s" % S)

MatrixMulAction.__init__(self, G, S, True)
Expand Down Expand Up @@ -304,7 +304,7 @@ cdef class MatrixVectorAction(MatrixMulAction):
...
TypeError: incompatible dimensions 3, 4
"""
if not is_FreeModule(S):
if not isinstance(S, FreeModule_generic):
raise TypeError("Not a free module: %s" % S)
MatrixMulAction.__init__(self, G, S, True)

Expand Down Expand Up @@ -355,7 +355,7 @@ cdef class VectorMatrixAction(MatrixMulAction):
...
TypeError: incompatible dimensions 5, 3
"""
if not is_FreeModule(S):
if not isinstance(S, FreeModule_generic):
raise TypeError("Not a free module: %s" % S)
MatrixMulAction.__init__(self, G, S, False)

Expand Down
2 changes: 1 addition & 1 deletion src/sage/matrix/matrix2.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -5786,7 +5786,7 @@ cdef class Matrix(Matrix1):
sage: t.decomposition_of_subspace(v, check_restrict=False) == t.decomposition_of_subspace(v) # needs sage.libs.pari
True
"""
if not sage.modules.free_module.is_FreeModule(M):
if not isinstance(M, sage.modules.free_module.FreeModule_generic):
raise TypeError("M must be a free module.")
if not self.is_square():
raise ArithmeticError("self must be a square matrix")
Expand Down
14 changes: 10 additions & 4 deletions src/sage/matrix/matrix_space.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,18 @@ def is_MatrixSpace(x):
sage: MS = MatrixSpace(QQ,2)
sage: A = MS.random_element()
sage: is_MatrixSpace(MS)
doctest:warning...
DeprecationWarning: the function is_MatrixSpace is deprecated;
use 'isinstance(..., MatrixSpace)' instead
See https://github.com/sagemath/sage/issues/37924 for details.
True
sage: is_MatrixSpace(A)
False
sage: is_MatrixSpace(5)
False
"""
from sage.misc.superseded import deprecation
deprecation(37924, "the function is_MatrixSpace is deprecated; use 'isinstance(..., MatrixSpace)' instead")
return isinstance(x, MatrixSpace)


Expand Down Expand Up @@ -1265,10 +1271,10 @@ def _get_action_(self, S, op, self_on_left):
if op is operator.mul:
from . import action as matrix_action
if self_on_left:
if is_MatrixSpace(S):
if isinstance(S, MatrixSpace):
# matrix multiplications
return matrix_action.MatrixMatrixAction(self, S)
elif sage.modules.free_module.is_FreeModule(S):
elif isinstance(S, sage.modules.free_module.FreeModule_generic):
return matrix_action.MatrixVectorAction(self, S)
elif isinstance(S, SchemeHomset_points):
return matrix_action.MatrixSchemePointAction(self, S)
Expand All @@ -1278,10 +1284,10 @@ def _get_action_(self, S, op, self_on_left):
# action of base ring
return sage.structure.coerce_actions.RightModuleAction(S, self)
else:
if is_MatrixSpace(S):
if isinstance(S, MatrixSpace):
# matrix multiplications
return matrix_action.MatrixMatrixAction(S, self)
elif sage.modules.free_module.is_FreeModule(S):
elif isinstance(S, sage.modules.free_module.FreeModule_generic):
return matrix_action.VectorMatrixAction(self, S)
elif isinstance(S, SchemeHomset_generic):
return matrix_action.PolymapMatrixAction(self, S)
Expand Down
4 changes: 2 additions & 2 deletions src/sage/modular/abvar/abvar.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
from sage.modular.modsym.modsym import ModularSymbols
from sage.modular.modsym.space import ModularSymbolsSpace
from sage.modular.quatalg.brandt import BrandtModule
from sage.modules.free_module import is_FreeModule
from sage.modules.free_module import FreeModule_generic
from sage.modules.free_module_element import vector
from sage.rings.fast_arith import prime_range
from sage.rings.infinity import infinity
Expand Down Expand Up @@ -4009,7 +4009,7 @@ def __init__(self, groups, lattice=None, base_field=QQ, is_simple=None, newform_
lattice = ZZ**(2 * self._ambient_dimension())
if check:
n = self._ambient_dimension()
if not is_FreeModule(lattice):
if not isinstance(lattice, FreeModule_generic):
raise TypeError("lattice must be a free module")
if lattice.base_ring() != ZZ:
raise TypeError("lattice must be over ZZ")
Expand Down
4 changes: 2 additions & 2 deletions src/sage/modular/abvar/finite_subgroup.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@
from sage.misc.lazy_import import lazy_import
from sage.modular.abvar.torsion_point import TorsionPoint
from sage.modules.module import Module
from sage.modules.free_module import is_FreeModule
from sage.modules.free_module import FreeModule_generic
from sage.structure.gens_py import abelian_iterator
from sage.structure.sequence import Sequence
from sage.structure.richcmp import richcmp_method, richcmp
Expand Down Expand Up @@ -871,7 +871,7 @@ def __init__(self, abvar, lattice, field_of_definition=None, check=True):
from sage.rings.qqbar import QQbar as field_of_definition
if check:
from .abvar import is_ModularAbelianVariety
if not is_FreeModule(lattice) or lattice.base_ring() != ZZ:
if not isinstance(lattice, FreeModule_generic) or lattice.base_ring() != ZZ:
raise TypeError("lattice must be a free module over ZZ")
if not is_ModularAbelianVariety(abvar):
raise TypeError("abvar must be a modular abelian variety")
Expand Down
4 changes: 2 additions & 2 deletions src/sage/modular/hecke/ambient_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from . import module
from . import submodule

from sage.modules.free_module import FreeModule, is_FreeModule
from sage.modules.free_module import FreeModule, FreeModule_generic
from sage.rings.integer import Integer

import sage.arith.all as arith
Expand Down Expand Up @@ -927,7 +927,7 @@ def submodule(self, M, Mdual=None, check=True):
Modular Forms subspace of dimension 2 of Modular Forms space of dimension 3 for Congruence Subgroup Gamma0(37) of weight 2 over Rational Field
"""
if check:
if not is_FreeModule(M):
if not isinstance(M, FreeModule_generic):
V = self.free_module()
if isinstance(M, (list, tuple)):
M = V.span([V(x.element()) for x in M])
Expand Down
8 changes: 4 additions & 4 deletions src/sage/modular/hecke/submodule.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import sage.arith.all as arith
from sage.misc.verbose import verbose
from sage.misc.cachefunc import cached_method
from sage.modules.free_module import is_FreeModule
from sage.modules.free_module import FreeModule_generic
from sage.structure.richcmp import richcmp_method, richcmp_not_equal

from . import module
Expand Down Expand Up @@ -81,7 +81,7 @@ def __init__(self, ambient, submodule, dual_free_module=None, check=True):
from . import ambient_module
if not isinstance(ambient, ambient_module.AmbientHeckeModule):
raise TypeError("ambient must be an ambient Hecke module")
if not is_FreeModule(submodule):
if not isinstance(submodule, FreeModule_generic):
raise TypeError("submodule must be a free module")
if not submodule.is_submodule(ambient.free_module()):
raise ValueError("submodule must be a submodule of the ambient free module")
Expand All @@ -96,7 +96,7 @@ def __init__(self, ambient, submodule, dual_free_module=None, check=True):
ambient.level(),
ambient.weight())
if dual_free_module is not None:
if not is_FreeModule(dual_free_module):
if not isinstance(dual_free_module, FreeModule_generic):
raise TypeError("dual_free_module must be a free module")
if dual_free_module.rank() != submodule.rank():
raise ArithmeticError("dual_free_module must have the same rank as submodule")
Expand Down Expand Up @@ -912,7 +912,7 @@ def submodule(self, M, Mdual=None, check=True):
sage: S.submodule(S[0].free_module())
Modular Symbols subspace of dimension 2 of Modular Symbols space of dimension 18 for Gamma_0(18) of weight 4 with sign 0 over Rational Field
"""
if not is_FreeModule(M):
if not isinstance(M, FreeModule_generic):
V = self.ambient_module().free_module()
if isinstance(M, (list, tuple)):
M = V.span([V(x.element()) for x in M])
Expand Down
4 changes: 2 additions & 2 deletions src/sage/modular/modsym/ambient.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class ``ModularSymbolsAmbient``, derived from
ManinSymbolList_gamma1,
ManinSymbolList_gamma_h,
ManinSymbolList_character)
from sage.modules.free_module import is_FreeModule
from sage.modules.free_module import FreeModule_generic
from sage.modules.free_module_element import FreeModuleElement
from sage.rings.integer import Integer
from sage.rings.integer_ring import ZZ
Expand Down Expand Up @@ -2133,7 +2133,7 @@ def submodule(self, M, dual_free_module=None, check=True):
Stein, 2007-07-27
"""
if check:
if not is_FreeModule(M):
if not isinstance(M, FreeModule_generic):
V = self.free_module()
if not isinstance(M, (list, tuple)):
M = M.gens()
Expand Down
30 changes: 18 additions & 12 deletions src/sage/modules/fg_pid/fgp_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@
from itertools import product

from sage.modules.module import Module
from sage.modules.free_module import is_FreeModule
from sage.modules.free_module import FreeModule_generic
from sage.structure.all import parent
from sage.structure.sequence import Sequence
from .fgp_element import DEBUG, FGP_Element
Expand Down Expand Up @@ -280,10 +280,16 @@ def is_FGP_Module(x):
sage: V = span([[1/2,1,1],[3/2,2,1],[0,0,1]],ZZ)
sage: W = V.span([2*V.0 + 4*V.1, 9*V.0 + 12*V.1, 4*V.2]); Q = V/W
sage: sage.modules.fg_pid.fgp_module.is_FGP_Module(V)
doctest:warning...
DeprecationWarning: the function is_FGP_Module is deprecated;
use 'isinstance(..., FGP_Module_class)' instead
See https://github.com/sagemath/sage/issues/37924 for details.
False
sage: sage.modules.fg_pid.fgp_module.is_FGP_Module(Q)
True
"""
from sage.misc.superseded import deprecation
deprecation(37924, "the function is_FGP_Module is deprecated; use 'isinstance(..., FGP_Module_class)' instead")
return isinstance(x, FGP_Module_class)


Expand Down Expand Up @@ -356,9 +362,9 @@ def __init__(self, V, W, check=True):
<class 'sage.modules.fg_pid.fgp_module.FGP_Module_class_with_category'>
"""
if check:
if not is_FreeModule(V):
if not isinstance(V, FreeModule_generic):
raise TypeError("V must be a FreeModule")
if not is_FreeModule(W):
if not isinstance(W, FreeModule_generic):
raise TypeError("W must be a FreeModule")
if not W.is_submodule(V):
raise ValueError("W must be a submodule of V")
Expand Down Expand Up @@ -441,7 +447,7 @@ def _coerce_map_from_(self, S):
sage: Q._coerce_map_from_(V.scale(2))
True
"""
if is_FGP_Module(S):
if isinstance(S, FGP_Module_class):
return S.has_canonical_map_to(self)
return self._V.has_coerce_map_from(S)

Expand Down Expand Up @@ -500,8 +506,8 @@ def __truediv__(self, other):
sage: Q/Q
Finitely generated module V/W over Integer Ring with invariants ()
"""
if not is_FGP_Module(other):
if is_FreeModule(other):
if not isinstance(other, FGP_Module_class):
if isinstance(other, FreeModule_generic):
other = other / other.zero_submodule()
else:
raise TypeError("other must be an FGP module")
Expand All @@ -525,7 +531,7 @@ def __eq__(self, other):
sage: Q == V/V.zero_submodule()
False
"""
if not is_FGP_Module(other):
if not isinstance(other, FGP_Module_class):
return False
return self._V == other._V and self._W == other._W

Expand Down Expand Up @@ -745,7 +751,7 @@ def submodule(self, x):
...
ValueError: x.V() must be contained in self's V.
"""
if is_FGP_Module(x):
if isinstance(x, FGP_Module_class):
if not x._W.is_submodule(self._W):
raise ValueError("x.W() must be contained in self's W.")

Expand All @@ -759,7 +765,7 @@ def submodule(self, x):
raise TypeError("x must be a list, tuple, or FGP module")

x = Sequence(x)
if is_FGP_Module(x.universe()):
if isinstance(x.universe(), FGP_Module_class):
# TODO: possibly inefficient in some cases
x = [self(v).lift() for v in x]
V = self._V.submodule(x) + self._W
Expand Down Expand Up @@ -791,7 +797,7 @@ def has_canonical_map_to(self, A):
False
"""
if not is_FGP_Module(A):
if not isinstance(A, FGP_Module_class):
return False
if self.cardinality() == 0 and self.base_ring() == A.base_ring():
return True
Expand Down Expand Up @@ -1598,7 +1604,7 @@ def hom(self, im_gens, codomain=None, check=True):
N = codomain
im_gens = Sequence(im_gens, universe=N)

if is_FreeModule(N):
if isinstance(N, FreeModule_generic):
# If im_smith_gens are not in an R-module, but are in a Free-module,
# then we quotient out by the 0 submodule and get an R-module.
N = FGP_Module(N, N.zero_submodule(), check=DEBUG)
Expand Down Expand Up @@ -2047,7 +2053,7 @@ def random_fgp_morphism_0(*args, **kwds):
sage: mor = fgp.random_fgp_morphism_0(4)
sage: mor.domain() == mor.codomain()
True
sage: fgp.is_FGP_Module(mor.domain())
sage: isinstance(mor.domain(), fgp.FGP_Module_class)
True
Each generator is sent to a random multiple of itself::
Expand Down
Loading

0 comments on commit 7935745

Please sign in to comment.