'term' and 'monomial' are inconsistently used in some Category and combinat code

[jbandlow]

I'm including a patch, but will rebase it against 4.3.1 once it is released.

CC: @sagetrac-sage-combinat

Component: categories

Author: Jason Bandlow

Reviewer: Nicolas M. Thiéry

Merged: sage-4.3.2.alpha0

Issue created by migration from https://trac.sagemath.org/ticket/7938

[jbandlow]

Attachment: swap_term_and_monomial-jb.patch.gz

[nthiery]

comment:2

This change has been discussed and approved on sage-devel and sage-combinat-devel. I went through the current patch and it looks good. This is a conditional positive review, pending a rebase on 4.3.1 (if at all needed), a recheck of all tests passing, and two little details:

• Renaming the patch to trac_7938_swap_term_and_monomial-jb.patch
• Adding a short description on top of it; say something like: "'term' and 'monomial' are inconsistently used in some Category and combinat code #7938: swap term and monomial in category and combinat code for consistency with the rest of sage"

Thanks Jason!

[nthiery]

Reviewer: Nicolas M. Thiéry

[nthiery]

comment:3

Replying to @nthiery:
Oops, I forgot the following:

• Removing the spurious change to sage/combinat/crystals/affine.py
• Adding a default value (the one of the coeff ring) for coeff in the new self.term, to make it backward compatible
• Only if easy, make the new monomial accept a second optional coeff argument, to make it backward compatible. This could be a bit tricky, since self.monomial is a Map. It is also not as important as for term, since I expect the later to have been used far more more extensively than the former.

[jbandlow]

comment:4

Attachment: trac_7938_swap_term_and_monomial-jb.patch.gz

The patch called 'trac_7938_...' is all that should be applied. In response to Nicolas' comments:

• Rename patch: done
• Add description: done
• Remove spurious change to affine.py: done, but see Cleanup of crystal code #7978
• Add default value for coeff in self.term: done
• Add optional coeff argument to monomial: Not done yet. I'll look more (but maybe not much more) at this later.

[nthiery]

comment:5

Replying to @jbandlow:

...

Thanks much!

• Add description: done

Sorry for bothering you again. Please also remove the [mq] line, and put the rest on one line (hg treats the first line specifically).

Cheers,

[nthiery]

comment:6

It does need a rebase w.r.t. #7729 (iwahori hecke algebra) whose file was renamed.

Please update the queue accordingly (including the #7729 patch).

[nthiery]

comment:7

Replying to @nthiery:

It does need a rebase w.r.t. #7729 (iwahori hecke algebra) whose file was renamed.

Please update the queue accordingly (including the #7729 patch).

Done!

[nthiery]

Rebased for 4.3.1

[nthiery]

comment:8

Attachment: trac_7938_swap_term_and_monomial-jb.2.patch.gz

Sorry for bothering you again. Please also remove the [mq] line, and put the rest on one line (hg treats the first line specifically).

Done. Patch ready for merging into Sage.

[sagetrac-mvngu]

Changed author from jbandlow to Jason Bandlow

[sagetrac-mvngu]

Merged: sage-4.3.2.alpha0

[sagetrac-mvngu]

comment:10

Merged trac_7938_swap_term_and_monomial-jb.2.patch.