#5734 closed defect (fixed)
[with patch, with positive review] Bring doctests of modular/modsym/manin_symbols.py up to 100%
Reported by: | John Cremona | Owned by: | Michael Abshoff |
---|---|---|---|
Priority: | major | Milestone: | sage-3.4.1 |
Component: | doctest coverage | Keywords: | modular symbols |
Cc: | David Loeffler, Maite Aranés | Merged in: | 3.4.1.rc3 |
Authors: | John Cremona | Reviewers: | David Loeffler |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
As of 3.4.1.rc1, we have
SCORE /home/jec/sage-3.4.1.rc1/devel/sage-main/sage/modular/modsym/manin_symbols.py: 2% (2 of 68) Missing documentation: * is_ManinSymbol(x): * __init__(self, weight, list): * __cmp__(self, right): * __getitem__(self, n): * __len__(self): * apply(self, j, X): * apply_S(self, j): * apply_I(self, j): * apply_T(self, j): * apply_TT(self, j): * manin_symbol_list(self): * manin_symbol(self, i): * normalize(self, x): * weight(self): * __init__(self, level, weight, syms): * apply_T(self, j): * apply_TT(self, j): * level(self): * normalize(self, x): * __init__(self, level, weight): * __repr__(self): * __init__(self, level, weight): * __repr__(self): * __init__(self, group, weight): * __repr__(self): * __init__(self, character, weight): * __repr__(self): * apply_T(self, j): * apply_TT(self, j): * character(self): * level(self): * normalize(self, x): * __init__(self, level, weight): * __repr__(self): * apply_T(self, j): * apply_TT(self, j): * level(self): * normalize(self, x): * tuple(self): * __get_i(self): * __get_u(self): * __get_v(self): * _repr_(self): * _latex_(self): * __cmp__(self, other): * __mul__(self, matrix): * copy(self): * parent(self): * weight(self): * _print_polypart(i, j): Missing doctests: * index(self, x): * apply_S(self, j): * apply_I(self, j): * apply(self, j, m): * apply_S(self, j): * apply_I(self, j): * index(self, x): * apply_S(self, j): * apply_I(self, j): * apply_J(self, j): * apply(self, j, m): * __init__(self, parent, t): * apply(self, a,b,c,d): * lift_to_sl2z(self, N): * endpoints(self, N=None): * modular_symbol_rep(self):
and I think I might have the right background to fix this!
Attachments (2)
Change History (9)
comment:1 Changed 14 years ago by
Cc: | David Loeffler Maite Aranés added |
---|
Changed 14 years ago by
Attachment: | trac_5734.patch added |
---|
comment:2 Changed 14 years ago by
Summary: | Bring doctests of modular/modsym/manin_symbols.py up to 100% → [with patch, needs review] Bring doctests of modular/modsym/manin_symbols.py up to 100% |
---|
The patch touches only sage/modular/modsym/manin_symbols.py, based on 3.4.1.rc2.
---------------------------------------------------------------------- sage/modular/modsym/manin_symbols.py SCORE sage/modular/modsym/manin_symbols.py: 100% (59 of 59) ----------------------------------------------------------------------
I also fixed some small bugs in functions which did not seem to be used, removed (by commenting out) a class x__ManinSymbolList_gamma1
which was not used, and implemented a slightly more sensible cmp() function for class ManinSymbol? (which used to return 1 for any two distinct symbols). All doctests in sage/modular pass, and I also checked quite thoroughly through the resulting reference manual page, which also looks good.
Review please!
comment:3 Changed 14 years ago by
Description: | modified (diff) |
---|
This looks great: applies fine to 3.4.1.rc2, all doctests pass, and documentation builds happily.
I've made a few slight adjustments to the formatting (mainly enforcing a consistent convention that INPUT: is always followed by a bulleted list and OUTPUT: never is, which was true for most but not all of the new doctests), and I've added an x == loads(dumps(x)) doctest for each of the classes that are actually intended for direct use. (There was a loads(dumps()) test in the file already, so sage -coverage didn't protest, but it was commented out, and it was for a different class anyway.)
John: if you're happy with the changes I've made, feel free to change this to "positive review".
comment:4 Changed 14 years ago by
Summary: | [with patch, needs review] Bring doctests of modular/modsym/manin_symbols.py up to 100% → [with patch, with positive review] Bring doctests of modular/modsym/manin_symbols.py up to 100% |
---|
I am happy. Thanks for going through this carefully. I also think that the conventions to be followed in these new-style docstrings should be written down somewhere!
comment:5 Changed 14 years ago by
Milestone: | sage-3.4.2 → sage-3.4.1 |
---|---|
Resolution: | → fixed |
Status: | new → closed |
Merged both patches in Sage 3.4.1.rc3.
Cheers,
Michael
comment:6 Changed 14 years ago by
Authors: | → John Cremona |
---|---|
Merged in: | → 3.4.1.rc3 |
Reviewers: | → David Loeffler |
comment:7 Changed 7 years ago by
Description: | modified (diff) |
---|---|
Report Upstream: | → N/A |
David, I thought you might like this one too (when I have done it) -J