[with patch, positive review] Labelling of newforms

[ljpk]

Given a space of CuspForms?, there is a newforms method which gives a list of newforms associated to that space, with a name specified by the user. However, this does not seem to work correctly at the moment:

sage: S=CuspForms(23)
sage: S.newforms('b')
[q + a0*q^2 + (-2*a0 - 1)*q^3 + (-a0 - 1)*q^4 + 2*a0*q^5 + O(q^6)]

I think that the newforms code should be changed to something like:

def newforms(self, names=None):
        """
        Return all cusp forms in the cuspidal subspace of self.
        
        EXAMPLES:
        
        sage: CuspForms(23).newforms('b')
        [q + b0*q^2 + (-2*b0 - 1)*q^3 + (-b0 - 1)*q^4 + 2*b0*q^5 + O(q^6)]
        """
        M = self.modular_symbols(sign=1)
        factors = M.cuspidal_subspace().new_subspace().decomposition()
        large_dims = [ X.dimension() for X in factors if X.dimension() != 1 ]
        if len(large_dims) > 0 and names is None:            
            names = 'a'
        return [ element.Newform(self, factors[i], names=(names+str(i)) )
                 for i in range(len(factors)) ]

(removing the ValueError? statement) as this should correctly use the user-specified name if one is given or default to 'a' if one is not.

[was]

Lloyd,

Thanks for pointing this out! I guess we only ever tested with 'a'. This will be good fodder for bug day on Thursday...

[craigcitro]

Sorry, I couldn't wait until Thursday. :) I think William's comment is right -- I probably just never tested this with anything except a, since that's the letter I usually use.

However, raising an error if no variable name is provided is, in fact, the intended behavior -- basically obeying the rule of "explicit is better than implicit."

[AlexGhitza]

Looks good to me.

[ljpk]

I personally find the fact that one has to manually assign a variable name really annoying, but if that's the design decision you've taken then fair enough (I can see your reasons; I just disagree).

[was]

I personally find the fact that one has to manually assign a variable name really annoying, but if that's the design decision you've taken then fair enough (I can see your reasons; I just disagree).

We've systematically made that decision throughout all Sage, so we should stick with that here.

That said, we have also talked about making it so one can specify a uniform default throughout sage, e.g., a function f(n) that takes as input an integer n and outputs a variable name. You could define it to be anything you want and everywhere in Sage that requires variables would call it -- if specified instead of giving an error, when you forget to give a variable name.

[craigcitro]

Good point. The request for a system-wide variable default is now trac #4345.

[mabshoff]

Unfortunately other patches mandate a rebase of this patch:

mabshoff@sage:/scratch/mabshoff/release-cycle/sage-3.2.alpha1/devel/sage$ patch -p1 --dry-run < trac_4335.patch 
patching file sage/modular/modform/space.py
Hunk #1 FAILED at 1571.
1 out of 1 hunk FAILED -- saving rejects to file sage/modular/modform/space.py.rej

Please try either my current merge tree on sage.math or alternatively wait for 3.2.alpha1 out in the next 12 hours

Cheers,

Michael

[craigcitro]

I rebased the patch, and it *should* work -- I don't have a current alpha, but I was pretty sure it was my patch from another ticket that caused the merge troubles. Let me know if this one doesn't work.

[mabshoff]

The rebased patch applies fine - now doctesting.

Cheers,

Michael

[mabshoff]

Merged in Sage 3.2.alpha1