modules with basis/map coefficients

[mantepse]

This is a replacement for #37271, without dependency on the construction functor for symmetric functions.

Fixes #18264

[tscrim]

Why is #37687 a dependency? This seems completely independent.

[tscrim]

As written, I don't think we want this error to be raised if the callable returns elements in, say ZZ but new_base_ring=GF(2). We want to automatically convert the elements for consistency:

sage: R.<x,y> = ZZ[]
sage: x.map_coefficients(lambda p: 2*p, GF(2))
0
sage: x.map_coefficients(lambda p: GF(2)(p), QQ)
x

I think what you meant to say is if the coefficients must be able to be converted into new_base_ring.

Additionally, if f is a map, then new_base_ring should still be the ultimate target ring for consistency:

sage: R.<a> = QQ[]
sage: f = R.hom([a^2])
sage: S.<x,y> = R[]
sage: (a*x).map_coefficients(f)
a^2*x
sage: (a*x).map_coefficients(f, R['q'])
a^2*x
sage: _.parent()
Multivariate Polynomial Ring in x, y over Univariate Polynomial Ring in q over Univariate Polynomial Ring in a over Rational Field

[mantepse]

Do I understand correctly that you are asking me to adapt the documentation, the code is doing what it should, right?

[tscrim]

Yes, that's correct.

[mantepse]

Why is #37687 a dependency? This seems completely independent.

The doctest only works with #37687.

[tscrim]

It would be better to use a doctest that does not depend on #37687. (In fact, this change is very mildly bad because it depends on something that is not implemented in the category.) Just use a regular CFM:

sage: M = CombinatorialFreeModule(ZZ, 'abc')

[mantepse]

It would be better to use a doctest that does not depend on #37687. (In fact, this change is very mildly bad because it depends on something that is not implemented in the category.) Just use a regular CFM:

sage: M = CombinatorialFreeModule(ZZ, 'abc')

OK

[mantepse]

Is this what you had in mind? (If so, I would then do yet another PR with the dependency removed. I'd also do the pycodestyle suggestions for the whole file, if that's ok.)

diff --git a/src/sage/categories/modules_with_basis.py b/src/sage/categories/modules_with_basis.py
index ef6e43c5b19..dcbd547fe53 100644
--- a/src/sage/categories/modules_with_basis.py
+++ b/src/sage/categories/modules_with_basis.py
@@ -2073,7 +2073,8 @@ class ModulesWithBasis(CategoryWithAxiom_over_base_ring):
             resulting polynomial will be over the same ring as ``self``. Set
             ``new_base_ring`` to override this behaviour.
 
-            An error is raised if the coefficients are not in the new base ring.
+            An error is raised if the coefficients cannot be
+            converted to the new base ring.
 
             INPUT:
 
@@ -2108,18 +2109,27 @@ class ModulesWithBasis(CategoryWithAxiom_over_base_ring):
 
             We can map into a different base ring::
 
-                sage: # needs sage.combinat
-                sage: e = SymmetricFunctions(QQ).elementary()
-                sage: a = 1/2*(e([2,1]) + e([1,1,1])); a
-                1/2*e[1, 1, 1] + 1/2*e[2, 1]
+                sage: F = CombinatorialFreeModule(QQ, ['a','b','c'])
+                sage: B = F.basis()
+                sage: a = 1/2*(B['a'] + 3*B['c']); a
+                1/2*B['a'] + 3/2*B['c']
                 sage: b = a.map_coefficients(lambda c: 2*c, ZZ); b
-                e[1, 1, 1] + e[2, 1]
+                B['a'] + 3*B['c']
                 sage: b.parent()
-                Symmetric Functions over Integer Ring in the elementary basis
+                Free module generated by {'a', 'b', 'c'} over Integer Ring
                 sage: b.map_coefficients(lambda c: 1/2*c, ZZ)
                 Traceback (most recent call last):
                 ...
                 TypeError: no conversion of this rational to integer
+
+            Coefficients are converted to the new base ring after
+            applying the map::
+
+                sage: B['a'].map_coefficients(lambda c: 2*c, GF(2))
+                0
+                sage: B['a'].map_coefficients(lambda c: GF(2)(c), QQ)
+                B['a']
+
             """
             R = self.parent()
             if isinstance(f, Map):

[tscrim]

Yes, that's what I was thinking. You don't need a new PR (it would make it much easier for me if you didn't); you can interactive rebase and just remove the commits from #37687 (among other ways) and force push the result.

[mantepse]

need more help with removing the offending commits, sorry.

[mantepse]

success!

[github-actions]

Documentation preview for this PR (built with commit ed68d68; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

[mkoeppe]

LGTM