Skip to content

Commit

Permalink
Trac #34509: Make IndexedFreeModuleElement compatible with collection…
Browse files Browse the repository at this point in the history
…s.abc, change method support to return a SupportView

This is the element class of a `CombinatorialFreeModule`.

Its implementation of `__iter__`, `__contains__`, `__len__` are not
compatible with anything in the [[https://docs.python.org/3/library/coll
ections.abc.html|collections.abc]] protocols. Specifically,
{{{
sage: F = CombinatorialFreeModule(QQ, ['a','b','c'])
sage: B = F.basis()
sage: f = B['a'] + 3*B['c']; f
sage: import collections.abc
sage: isinstance(f, collections.abc.Collection)
True
}}}
but `__iter__`ating over this collection gives objects that are not
`__contain__`ed in it.

To improve interoperability, we propose to make the following changes:
 - Deprecate the `__contains__` method (currently testing whether a
given index is in the support). After its removal, the class would no
longer be considered by Python to be a subclass of
`collections.abc.Collection`, but merely a `Sized` `Iterable`.
 - As a replacement for this functionality, revise the method `support`
to return an instance of a new class `SupportView`, which provides a
fast `__contains__` method, instead of a list.

(Similarly, in #24815, it was proposed to make the elements of free
modules from `sage.modules` a [[https://docs.python.org/3/library/collec
tions.abc.html#collections.abc.Sequence|collections.abc.Sequence]].)

Part of Meta-ticket #30309: Unify free module elements API: methods
dict, monomial_coefficients, etc.

URL: https://trac.sagemath.org/34509
Reported by: mkoeppe
Ticket author(s): Matthias Koeppe
Reviewer(s): Travis Scrimshaw
  • Loading branch information
Release Manager committed Sep 20, 2022
2 parents 0b597a7 + 2fe3b98 commit 4db51ba
Show file tree
Hide file tree
Showing 16 changed files with 246 additions and 38 deletions.
6 changes: 3 additions & 3 deletions build/pkgs/configure/checksums.ini
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
tarball=configure-VERSION.tar.gz
sha1=6f8c2081de6c724babc10f767d2090e873fc5c8a
md5=8e938868a2d0af606afacc8aa3b4de59
cksum=3217316133
sha1=2b72df4c224b120993d18936397bcd73c04c5b6d
md5=0ea8af9d5139c7d024a0d3153436474c
cksum=1903922458
2 changes: 1 addition & 1 deletion build/pkgs/configure/package-version.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
60e5acf5109db9dc40877bd62fad4556b85b8c1d
9d8ff5ded968b56356f5029460efbbd83c56cdf3
4 changes: 2 additions & 2 deletions src/doc/en/thematic_tutorials/lie/weyl_character_ring.rst
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ coefficients) through the usual free module accessors::
[((0, 0, 0), 1), ((1, 0, 0), 1), ((1, 1, 0), 1), ((1, 1, 1), 1)]
sage: pprint(dict(chi))
{(0, 0, 0): 1, (1, 0, 0): 1, (1, 1, 0): 1, (1, 1, 1): 1}
sage: M = sorted(chi.monomials(), key=lambda x: x.support()); M
sage: M = sorted(chi.monomials(), key=lambda x: tuple(x.support())); M
[B3(0,0,0), B3(1,0,0), B3(1,1,0), B3(1,1,1)]
sage: sorted(chi.support())
[(0, 0, 0), (1, 0, 0), (1, 1, 0), (1, 1, 1)]
Expand Down Expand Up @@ -485,7 +485,7 @@ itself, that is, the integral of `|tr(g)|^{10}`::

sage: tr^5
5*A2(2,2,1) + 6*A2(3,1,1) + 5*A2(3,2,0) + 4*A2(4,1,0) + A2(5,0,0)
sage: sorted((tr^5).monomials(), key=lambda x: x.support())
sage: sorted((tr^5).monomials(), key=lambda x: tuple(x.support()))
[A2(2,2,1), A2(3,1,1), A2(3,2,0), A2(4,1,0), A2(5,0,0)]
sage: sorted((tr^5).coefficients())
[1, 4, 5, 5, 6]
Expand Down
10 changes: 4 additions & 6 deletions src/sage/algebras/quantum_groups/fock_space.py
Original file line number Diff line number Diff line change
Expand Up @@ -1354,9 +1354,8 @@ def _G_to_fock_basis(self, la):
return fock.sum_of_terms((fock._indices([[]]*k + list(pt)), c) for pt,c in cur)

cur = R.A()._A_to_fock_basis(la)
s = cur.support()
s.sort() # Sort lex, which respects dominance order
s.pop() # Remove the largest
s = sorted(cur.support()) # Sort lex, which respects dominance order
s.pop() # Remove the largest

q = R._q
while s:
Expand Down Expand Up @@ -2189,9 +2188,8 @@ def add_cols(nu):

# Perform the triangular reduction
cur = self.realization_of().A(algorithm)._A_to_fock_basis(la)
s = cur.support()
s.sort() # Sort lex, which respects dominance order
s.pop() # Remove the largest
s = sorted(cur.support()) # Sort lex, which respects dominance order
s.pop() # Remove the largest

q = self.realization_of()._q
while s:
Expand Down
8 changes: 4 additions & 4 deletions src/sage/algebras/steenrod/steenrod_algebra.py
Original file line number Diff line number Diff line change
Expand Up @@ -402,13 +402,13 @@
(1, (2, 1))
sage: c.monomial_coefficients() == {(2, 1): 1, (5,): 1}
True
sage: sorted(c.monomials(), key=lambda x: x.support())
sage: sorted(c.monomials(), key=lambda x: tuple(x.support()))
[Sq(2,1), Sq(5)]
sage: sorted(c.support())
[(2, 1), (5,)]
sage: Adem = SteenrodAlgebra(basis='adem')
sage: elt = Adem.Sq(10) + Adem.Sq(9) * Adem.Sq(1)
sage: sorted(elt.monomials(), key=lambda x: x.support())
sage: sorted(elt.monomials(), key=lambda x: tuple(x.support()))
[Sq^9 Sq^1, Sq^10]
sage: A7 = SteenrodAlgebra(p=7)
Expand Down Expand Up @@ -3100,7 +3100,7 @@ class Element(CombinatorialFreeModule.Element):
(1, (2, 1))
sage: c.monomial_coefficients() == {(2, 1): 1, (5,): 1}
True
sage: sorted(c.monomials(), key=lambda x: x.support())
sage: sorted(c.monomials(), key=lambda x: tuple(x.support()))
[Sq(2,1), Sq(5)]
sage: sorted(c.support())
[(2, 1), (5,)]
Expand Down Expand Up @@ -3458,7 +3458,7 @@ def excess(self):
sage: (Sq(0,0,1) + Sq(4,1) + Sq(7)).excess()
1
sage: elt = Sq(0,0,1) + Sq(4,1) + Sq(7)
sage: M = sorted(elt.monomials(), key=lambda x: x.support())
sage: M = sorted(elt.monomials(), key=lambda x: tuple(x.support()))
sage: [m.excess() for m in M]
[1, 5, 7]
sage: [m for m in M]
Expand Down
15 changes: 8 additions & 7 deletions src/sage/categories/crystals.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
# https://www.gnu.org/licenses/
#*****************************************************************************

import collections.abc

from sage.misc.cachefunc import cached_method
from sage.misc.abstract_method import abstract_method
Expand Down Expand Up @@ -779,10 +780,10 @@ def crystal_morphism(self, on_gens, codomain=None,
if codomain is None:
if hasattr(on_gens, 'codomain'):
codomain = on_gens.codomain()
elif isinstance(on_gens, (list, tuple)):
elif isinstance(on_gens, collections.abc.Sequence):
if on_gens:
codomain = on_gens[0].parent()
elif isinstance(on_gens, dict):
elif isinstance(on_gens, collections.abc.Mapping):
if on_gens:
codomain = next(iter(on_gens.values())).parent()
else:
Expand Down Expand Up @@ -1845,7 +1846,7 @@ def __init__(self, parent, cartan_type=None,
scaling_factors = {i: 1 for i in index_set}
if virtualization is None:
virtualization = {i: (i,) for i in index_set}
elif not isinstance(virtualization, dict):
elif not isinstance(virtualization, collections.abc.Mapping):
try:
virtualization = dict(virtualization)
except (TypeError, ValueError):
Expand Down Expand Up @@ -2057,16 +2058,16 @@ def __init__(self, parent, on_gens, cartan_type=None,
virtualization, scaling_factors)

if gens is None:
if isinstance(on_gens, dict):
if isinstance(on_gens, collections.abc.Mapping):
gens = on_gens.keys()
else:
gens = parent.domain().module_generators
self._gens = tuple(gens)

# Make sure on_gens is a function
if isinstance(on_gens, dict):
if isinstance(on_gens, collections.abc.Mapping):
f = lambda x: on_gens[x]
elif isinstance(on_gens, (list, tuple)):
elif isinstance(on_gens, collections.abc.Sequence):
if len(self._gens) != len(on_gens):
raise ValueError("invalid generator images")
d = {x: y for x, y in zip(self._gens, on_gens)}
Expand Down Expand Up @@ -2579,7 +2580,7 @@ def __call__(self, on_gens, cartan_type=None, index_set=None, generators=None,
if automorphism is not None:
if virtualization is not None:
raise ValueError("the automorphism and virtualization cannot both be specified")
if not isinstance(automorphism, dict):
if not isinstance(automorphism, collections.abc.Mapping):
try:
automorphism = dict(automorphism)
virtualization = {i: (automorphism[i],) for i in automorphism}
Expand Down
2 changes: 1 addition & 1 deletion src/sage/categories/loop_crystals.py
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ def b_sharp(self):
bsharp = None
for b in self:
phi = b.Phi()
if phi.support() == [0] and phi[0] < ell:
if list(phi.support()) == [0] and phi[0] < ell:
bsharp = b
ell = phi[0]
return bsharp
Expand Down
22 changes: 15 additions & 7 deletions src/sage/categories/modules_with_basis.py
Original file line number Diff line number Diff line change
Expand Up @@ -1506,9 +1506,7 @@ def __len__(self):
sage: len(z)
4
"""
zero = self.parent().base_ring().zero()
return len([key for key, coeff in self.monomial_coefficients(copy=False).items()
if coeff != zero])
return len(self.support())

def length(self):
"""
Expand All @@ -1534,7 +1532,7 @@ def length(self):

def support(self):
"""
Return a list of the objects indexing the basis of
Return an iterable of the objects indexing the basis of
``self.parent()`` whose corresponding coefficients of
``self`` are non-zero.
Expand All @@ -1555,9 +1553,19 @@ def support(self):
sage: sorted(z.support())
[[1], [1, 1, 1], [2, 1], [4]]
"""
zero = self.parent().base_ring().zero()
return [key for key, coeff in self.monomial_coefficients(copy=False).items()
if coeff != zero]
try:
return self._support_view
except AttributeError:
from sage.structure.support_view import SupportView
zero = self.parent().base_ring().zero()
mc = self.monomial_coefficients(copy=False)
support_view = SupportView(mc, zero=zero)
try:
# Try to cache it for next time, but this may fail for Cython classes
self._support_view = support_view
except AttributeError:
pass
return support_view

def monomials(self):
"""
Expand Down
4 changes: 3 additions & 1 deletion src/sage/combinat/crystals/subcrystal.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
# http://www.gnu.org/licenses/
#****************************************************************************

import collections.abc

from sage.misc.lazy_attribute import lazy_attribute
from sage.structure.unique_representation import UniqueRepresentation
from sage.structure.parent import Parent
Expand Down Expand Up @@ -126,7 +128,7 @@ def __classcall_private__(cls, ambient, contained=None, generators=None,
sage: S1 is S2
True
"""
if isinstance(contained, (list, tuple, set, frozenset)):
if isinstance(contained, (collections.abc.Sequence, collections.abc.Set)):
contained = frozenset(contained)
#elif contained in Sets():

Expand Down
2 changes: 1 addition & 1 deletion src/sage/combinat/ncsf_qsym/generic_basis_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ def skew(self, x, y, side='left'):
y = self.dual()(y)
v = 1 if side == 'left' else 0
return self.sum(coeff * y[IJ[1-v]] * self[IJ[v]] \
for (IJ, coeff) in x.coproduct() if IJ[1-v] in y)
for (IJ, coeff) in x.coproduct() if IJ[1-v] in y.support())
else:
return self._skew_by_coercion(x, y, side=side)

Expand Down
2 changes: 1 addition & 1 deletion src/sage/combinat/ncsf_qsym/tutorial.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@
sage: sorted(z.coefficients())
[1, 2, 3]
sage: sorted(z.monomials(), key=lambda x: x.support())
sage: sorted(z.monomials(), key=lambda x: tuple(x.support()))
[M[1, 2], M[3, 3], M[6]]
sage: z.monomial_coefficients()
Expand Down
2 changes: 1 addition & 1 deletion src/sage/combinat/sf/character.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def _other_to_self(self, sexpr):
"""
if sexpr == 0:
return self(0)
if sexpr.support() == [[]]:
if list(sexpr.support()) == [[]]:
return self._from_dict({self.one_basis(): sexpr.coefficient([])},
remove_zeros=False)
out = self.zero()
Expand Down
2 changes: 1 addition & 1 deletion src/sage/combinat/sf/sfa.py
Original file line number Diff line number Diff line change
Expand Up @@ -2601,7 +2601,7 @@ def _inner_plethysm_pk_g(self, k, g, cache):
for d in degrees:
for mu in Partitions_n(d):
mu_k = mu.power(k)
if mu_k in g:
if mu_k in g.support():
res += g.coefficient(mu_k)*mu_k.centralizer_size()/mu.centralizer_size()*p(mu)

cache[(k,g)] = res
Expand Down
2 changes: 1 addition & 1 deletion src/sage/modules/tutorial_free_modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@
(2, 3)
sage: f.support()
[0, 1, 2]
SupportView({0: 2, 1: 2, 2: 3})
sage: f.monomials()
[a[0], a[1], a[2]]
sage: f.coefficients()
Expand Down
24 changes: 23 additions & 1 deletion src/sage/modules/with_basis/indexed_element.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,31 @@ from cpython.object cimport Py_NE, Py_EQ
from sage.misc.repr import repr_lincomb
from sage.misc.cachefunc import cached_method
from sage.misc.lazy_attribute import lazy_attribute
from sage.misc.superseded import deprecation
from sage.typeset.ascii_art import AsciiArt, empty_ascii_art, ascii_art
from sage.typeset.unicode_art import UnicodeArt, empty_unicode_art, unicode_art
from sage.categories.all import Category, Sets, ModulesWithBasis
from sage.data_structures.blas_dict cimport add, negate, scal, axpy


cdef class IndexedFreeModuleElement(ModuleElement):
r"""
Element class for :class:`~sage.combinat.free_module.CombinatorialFreeModule`
TESTS::
sage: import collections.abc
sage: F = CombinatorialFreeModule(QQ, ['a','b','c'])
sage: B = F.basis()
sage: f = B['a'] + 3*B['c']; f
B['a'] + 3*B['c']
sage: isinstance(f, collections.abc.Sized)
True
sage: isinstance(f, collections.abc.Iterable)
True
sage: isinstance(f, collections.abc.Collection) # known bug - will be fixed by removing __contains__
False
"""
def __init__(self, M, x):
"""
Create a combinatorial module element.
Expand Down Expand Up @@ -80,6 +98,9 @@ cdef class IndexedFreeModuleElement(ModuleElement):
sage: B = F.basis()
sage: f = B['a'] + 3*B['c']
sage: 'a' in f
doctest:warning...
DeprecationWarning: using 'index in vector' is deprecated; use 'index in vector.support()' instead
See https://trac.sagemath.org/34509 for details.
True
sage: 'b' in f
False
Expand All @@ -91,7 +112,8 @@ cdef class IndexedFreeModuleElement(ModuleElement):
sage: Partition([1,1,1]) in a
False
"""
return x in self._monomial_coefficients and self._monomial_coefficients[x] != 0
deprecation(34509, "using 'index in vector' is deprecated; use 'index in vector.support()' instead")
return x in self.support()

def __hash__(self):
"""
Expand Down
Loading

0 comments on commit 4db51ba

Please sign in to comment.