Skip to content

Consistent variable names for rings #40518

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/sage/algebras/lie_algebras/lie_algebra.py
Original file line number Diff line number Diff line change
Expand Up @@ -835,12 +835,12 @@ def gens(self) -> tuple:
(x, y)
"""
G = self.lie_algebra_generators()
if not self.variable_names():
return tuple(G)
try:
return tuple(G[i] for i in self.variable_names())
except (KeyError, IndexError):
return tuple(G[i] for i in self.indices())
except ValueError:
return tuple(G)

def gen(self, i):
"""
Expand Down
18 changes: 11 additions & 7 deletions src/sage/algebras/lie_algebras/quotient.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,13 +227,17 @@ def __classcall_private__(cls, ambient, I, names=None, index_set=None,
index_set = [i[0] for i in index_set_mapping if i[0] not in I_supp]

if names is None:
try:
amb_names = dict(zip([i[1] for i in index_set_mapping], ambient.variable_names()))
names = [amb_names[i] for i in index_set]
except (ValueError, KeyError):
# ambient has not assigned variable names
# or the names are for the generators rather than the basis
names = 'e'
names = 'e'
if ambient.variable_names():
# ambient has not assigned variable names...
try:
amb_names = dict(zip([i[1] for i in index_set_mapping],
ambient.variable_names()))
names = [amb_names[i] for i in index_set]
except KeyError:
# or the names are for the generators rather than
# the basis.
pass
if isinstance(names, str):
if len(index_set) == 1:
names = [names]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,7 @@ def _latex_(self):
if self.is_zero():
return "0"
p = self.parent()
try:
names = p.latex_variable_names()
except ValueError:
names = None
names = p.latex_variable_names()
if names:
terms = [("T^{{({})}}{}".format(k1, names[p._index_to_pos[k0]]), v) if k1 > 1
else ("T{}".format(names[p._index_to_pos[k0]]), v) if k1 == 1
Expand Down
8 changes: 1 addition & 7 deletions src/sage/groups/finitely_presented.py
Original file line number Diff line number Diff line change
Expand Up @@ -751,10 +751,6 @@ def __init__(self, free_group, relations, category=None, libgap_fpgroup=None):
<fp group of size 60 on the generators [ A_5.1, A_5.2 ]>
sage: A5sage = A5gapfp.sage(); A5sage;
Finitely presented group < A_5.1, A_5.2 | A_5.1^5*A_5.2^-5, A_5.1^5*(A_5.2^-1*A_5.1^-1)^2, (A_5.1^-2*A_5.2^2)^2 >
sage: A5sage.inject_variables()
Traceback (most recent call last):
...
ValueError: variable names have not yet been set using self._assign_names(...)

Check that pickling works::

Expand All @@ -772,10 +768,8 @@ def __init__(self, free_group, relations, category=None, libgap_fpgroup=None):
assert isinstance(relations, tuple)
self._free_group = free_group
self._relations = relations
try:
if free_group.variable_names():
self._assign_names(free_group.variable_names())
except ValueError:
pass
if libgap_fpgroup is None:
libgap_fpgroup = free_group.gap() / libgap([rel.gap() for rel in relations])
ParentLibGAP.__init__(self, libgap_fpgroup)
Expand Down
5 changes: 2 additions & 3 deletions src/sage/modules/free_module_element.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -3034,9 +3034,8 @@ cdef class FreeModuleElement(Vector): # abstract base class
[(x, y, z) |--> x, (x, y, z) |--> y, (x, y, z) |--> z]
"""
R = self._parent.base_ring()
try:
var_names = R.variable_names()
except ValueError:
var_names = R.variable_names()
if not var_names:
if hasattr(R, 'arguments'):
var_names = R.arguments()
else:
Expand Down
11 changes: 7 additions & 4 deletions src/sage/rings/asymptotic/misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,13 @@ def abbreviate(P):
op, cl = ('[', ']')
else:
op, cl = ('[[', ']]')
try:
s = abbreviate(P.base_ring()) + op + ', '.join(P.variable_names()) + cl
except ValueError:
s = str(P)
s = str(P)
if P.variable_names():
try:
s = abbreviate(P.base_ring()) + op + ', '.join(P.variable_names()) + cl
except ValueError:
pass

else:
try:
s = abbreviate(P)
Expand Down
6 changes: 5 additions & 1 deletion src/sage/rings/integer_ring.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -319,10 +319,14 @@ cdef class IntegerRing_class(CommutativeRing):

sage: A in InfiniteEnumeratedSets()
True

sage: ZZ.variable_names()
()

"""
cat = (EuclideanDomains(), DedekindDomains(),
InfiniteEnumeratedSets().Metric(), NoetherianRings())
Parent.__init__(self, base=self, names=('x',), normalize=False,
Parent.__init__(self, base=self, normalize=False,
category=cat)
self._populate_coercion_lists_(init_no_parent=True,
convert_method_name='_integer_')
Expand Down
42 changes: 7 additions & 35 deletions src/sage/rings/qqbar.py
Original file line number Diff line number Diff line change
Expand Up @@ -1075,40 +1075,6 @@ class AlgebraicRealField(Singleton, AlgebraicField_common, sage.rings.abc.Algebr
True
"""

def __new__(cls):
r"""
This method is there to ensure that pickles created before this class
was made a :class:`~sage.misc.fast_methods.Singleton` still load.

TESTS::

sage: s = loads(b'x\x9cmQ\xcbR\x141\x14\xad\x11A\x083\xe2\x03T|'
....: b'\x82l`\xd3\xff\xe0\x86\x8de/\xba*\xcb\xa9[\xe9\xf4'
....: b'\xa5;e:=\'I+,\xa6J\x17B\xf9\xd7f\x08\xe2s\x95\xa4\xee9\xf7<'
....: b'\xf2\xe5\x8e\x0e\xaa\xe5"D?\xea8z.\x9a\x0b\xa7z\xa3I[\x15'
....: b'\x82\xf8\xf3\x85\xc9\xb1<xg[\xae\xbd2\xbabeO\r\xdb\x86>\x9b'
....: b'\xd8\x91V\x91\xdb\xc1_\xe0f\xa57\xae\r\x05P+/\xfe\xe5\x08'
....: b'\xaci\xa2z46\x1aG$Z\x8e*F/p\xf7oC\xa33\x18\x99</<\x07v\tf'
....: b'\x06\'F\xe7\xb9\x195\x0b\xacg\xc2\x8d\xbc\xe1P\x9c\xad\x04'
....: b'\x828\xcd\x076N\x96W\xb8WaSN\x17\xca\xa7\r9\r\xb6.+\x88Kl'
....: b'\x97e\xb7\x16+LO\xbeb\xb6\xc4\xfdc)\x88\xfb\x9a\x9b&\x05'
....: b'\xc0N)wI\x0f\xee\x13\xfbH=\xc7nh(U\xc2xP\xca\r\xd2\x8d'
....: b'\x8a\n\x0fK\xb9\xf5+\xfe\xa3n3MV\x98\x80\xc7rr\xfe\r\xbbr'
....: b'\x9bZv\xecU\x1c|\xc0\xde\x12O\xe4:\xd5*0\x9ev3\xb9C\x0b'
....: b'\xa3?Z\xa6\xa4\x11R6<{?I\xa2l\xb9\xbf6;\xb8\\\xc6\xe0\xb1'
....: b'\x9f\xb3\xf6&\xe8\xe2,\xb3R\x13\xf9\xf2\xe1\xda\x9c\xc0s'
....: b'\xb9\xf7?.\xe1E7\xeb\xa6W\x15^&\x80q&\x1aeo\x93Y\x13"^\xcd'
....: b'\xf1Z\xee\xdf\x92W\x18Z\xa4\xa6(\xd7\x867\xdf\x93\xad\x9fL'
....: b'\xa5W\xff\x90\x89\x07s\x1c\xfe6\xd2\x03{\xcdy\xf4v\x8e\xa3'
....: b'\xb1.~\x000\xc2\xe0\xa1')
sage: s is AA
True
Comment on lines -1085 to -1105
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you can remove this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(The commit message mentions this, so apologies if you have read it already.)

The reason I've removed this test is because it is loading a copy of AA from before 2014, and AA is a singleton, so loading the old copy puts x back into the list of variables. The test for AA having no variables then of course fails. Likewise,if any other tests are added for bugfixes to AA in the future, they may mysteriously fail after the modern AA is overwritten.

I think it is pretty unlikely that someone will want to unpickle a copy of AA from before 2014, especially considering that (as evidenced by the test failures) it can bring back old bugs and incompatibilities. For a reference point, we switched to python-3.x in 2020.

FWIW I could probably hack the _names after this test to keep it working for now, but I don't feel great about loading a mysterious binary string into scope and then using it for the rest of the test suite without knowing what the differences are compare to the modern AA.

"""
try:
return AA
except BaseException:
return AlgebraicField_common.__new__(cls)

def __init__(self):
r"""
Standard initialization function.
Expand All @@ -1126,9 +1092,15 @@ def __init__(self):
True
sage: AA.has_coerce_map_from(int)
True

TESTS::

sage: AA.variable_names()
()

"""
from sage.categories.fields import Fields
AlgebraicField_common.__init__(self, self, ('x',), normalize=False, category=Fields().Infinite())
AlgebraicField_common.__init__(self, self, normalize=False, category=Fields().Infinite())
self._populate_coercion_lists_([ZZ, QQ])

def _element_constructor_(self, x):
Expand Down
16 changes: 6 additions & 10 deletions src/sage/rings/quotient_ring.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,10 +296,8 @@ def QuotientRing(R, I, names=None, **kwds):
raise TypeError("R must be a ring")
is_commutative = R in _CommRings
if names is None:
try:
names = tuple([x + 'bar' for x in R.variable_names()])
except ValueError: # no names are assigned
pass
if R.variable_names():
names = tuple(x + 'bar' for x in R.variable_names())
else:
names = normalize_names(R.ngens(), names)
if kwds.get('implementation') == 'pbori':
Expand Down Expand Up @@ -549,12 +547,10 @@ def construction(self):

# Is there a better generic way to distinguish between things like Z/pZ as a field and Z/pZ as a ring?
from sage.rings.ring import Field
try:
names = self.variable_names()
except ValueError:
try:
names = self.cover_ring().variable_names()
except ValueError:
names = self.variable_names()
if not names:
names = self.cover_ring().variable_names()
if not names:
names = None
if self in _CommRings:
return QuotientFunctor(self.__I, names=names, domain=_CommRings,
Expand Down
18 changes: 8 additions & 10 deletions src/sage/rings/quotient_ring_element.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,11 +220,10 @@ def _repr_(self):
# However, it may occur that no variable names are assigned.
# That holds, in particular, if there are infinitely many
# generators, as for Steenrod algebras.
try:
P.variable_names()
except ValueError:
return str(self.__rep)
with localvars(R, P.variable_names(), normalize=False):
if P.variable_names():
with localvars(R, P.variable_names(), normalize=False):
return str(self.__rep)
else:
return str(self.__rep)

def _latex_(self):
Expand All @@ -245,11 +244,10 @@ def _latex_(self):
P = self.parent()
R = P.cover_ring()
# see _repr_ above for the idea
try:
P.variable_names()
except ValueError:
return self.__rep._latex_()
with localvars(R, P.variable_names(), normalize=False):
if P.variable_names():
with localvars(R, P.variable_names(), normalize=False):
return self.__rep._latex_()
else:
return self.__rep._latex_()

def __pari__(self):
Expand Down
7 changes: 3 additions & 4 deletions src/sage/rings/rational_field.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,10 +219,10 @@ def __init__(self):
TESTS::

sage: TestSuite(QQ).run()
sage: QQ.variable_name()
'x'

sage: QQ.variable_names()
('x',)
()

sage: QQ._element_constructor_((2, 3))
2/3

Expand All @@ -237,7 +237,6 @@ def __init__(self):
Parent.__init__(self, base=self,
category=[QuotientFields().Metric(),
NumberFields()])
self._assign_names(('x',), normalize=False) # ?????
self._populate_coercion_lists_(init_no_parent=True)

_element_constructor_ = Rational
Expand Down
24 changes: 15 additions & 9 deletions src/sage/structure/category_object.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -462,9 +462,11 @@ cdef class CategoryObject(SageObject):

def variable_names(self):
"""
Return the list of variable names corresponding to the generators.
Return the variable names corresponding to our generators.

OUTPUT: a tuple of strings
OUTPUT:

A (possibly empty) tuple of strings.

EXAMPLES::

Expand All @@ -480,10 +482,16 @@ cdef class CategoryObject(SageObject):
sage: T.<x> = InfinitePolynomialRing(ZZ)
sage: T.variable_names()
('x',)

::

sage: NN.variable_names()
()

"""
if self._names is not None:
return self._names
raise ValueError("variable names have not yet been set using self._assign_names(...)")
return ()

def variable_name(self):
"""
Expand Down Expand Up @@ -525,13 +533,11 @@ cdef class CategoryObject(SageObject):
# We cannot assume that self *has* _latex_variable_names.
# But there is a method that returns them and sets
# the attribute at the same time, if needed.
# Simon King: It is not necessarily the case that variable
# names are assigned. In that case, self._names is None,
# and self.variable_names() raises a ValueError
try:

old = None, None
if self.variable_names():
old = self.variable_names(), self.latex_variable_names()
except ValueError:
old = None, None

self._names, self._latex_names = names, latex_names
return old

Expand Down
Loading