[with patch, positive review (issues have been addressed)] Wrap Cremona's newforms class

[boothby]

Wrap the newforms class in eclib. Patches will depend upon an updated eclib.

Component: modular forms

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

[sagetrac-mabshoff]

comment:4

Three remarks:

• I am splitting the updated eclib.spkg off into its own ticket better parsing for symbolics #2347
• doctests seems to be missing
• It would be interesting to see if this conflicts with wstein's mwrank rewrite

Cheers,

Michael

[sagetrac-mabshoff]

comment:5

I fail to see any difference to eclib-20080127.p0 in the hg log, so can somebody enlighten me what is different?

Cheers,

Michael

[JohnCremona]

comment:6

mabshoff is right -- this is the same as eclib-20080127. I think boothby forgot to check in his changes?

[sagetrac-mabshoff]

comment:7

Yep. Checking the repo I see:

eclib-20080304/src$ hg status
M g0n/Makefile
M g0n/hecketest.cc
M g0n/homspace.cc
M g0n/homspace.h
M g0n/newforms.cc
M g0n/newforms.h
M g0n/nftest.cc
M procs/rat.h
? g0n/ecnf.cc

Cheers,

Michael

[boothby]

comment:8

I didn't change the title of this ticket for a reason! Nothing here is ready for review. The eclib spkg has bad changes, like I made some private class members public, etc. The patch (as noted) doesn't have doctests, and doesn't work. I put this up as a preliminary version for William to work with. Later this week, I'll make it all kosher.

[boothby]

comment:9

See #2437 (not 2347)

[boothby]

Attachment: 2394-newforms.patch.gz

[malb]

comment:11

patch looks good, except:

• newly created files don't have file level documentation
• newly created files don't have copyright statement (do they need one?)
• Does it make sense to add doctests to class ECModularSymbol ?

[JohnCremona]

comment:12

I also think the patch looks good, though I agree with malb's points. Also I have no experience myself in wrapping code (even my own ;)) so the fact that it looks ok to me does not count for much.

Since this patch has required a little tinkering with eclib itself, I think I need to download that, compare with the latest version of my own, and check that nothing is broken...

[boothby]

comment:13

Attachment: 2394-license.patch.gz

2394-license.patch adds a copyright statement. File-level documentation would be redundant since the file only has a single class, and the class is documented. Also, ECModularSymbol has doctests on every function. Should we add more? If so, what? Modulo any further complaints, we should add this in to avoid bitrot.

[malb]

comment:14

[22:00] <boothby> Please look at my comments on #2394
[22:01] <boothby> valid/invalid?
[22:02] <malb> I'm still in favour of module level docs but that is taste I reckon

I say apply.

[sagetrac-mabshoff]

comment:15

Merged both patches in Sage 3.0.alpha4