Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parse_label() in sage/modular/modform/constructor.py fails in GammaH case #20823

Closed
kevinywlui mannequin opened this issue Jun 14, 2016 · 21 comments
Closed

parse_label() in sage/modular/modform/constructor.py fails in GammaH case #20823

kevinywlui mannequin opened this issue Jun 14, 2016 · 21 comments

Comments

@kevinywlui
Copy link
Mannequin

kevinywlui mannequin commented Jun 14, 2016

Currently, it does not return index in the GammaH case though the Gamma0 and Gamma1 case does.

In Newform(), both the group and the index is expected L485 in sage/modular/modform/constructor.py

Component: modular forms

Keywords: parse_label, modular_form

Author: Kevin Lui

Branch/Commit: b00abe1

Reviewer: William Stein, Frédéric Chapoton

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

@kevinywlui kevinywlui mannequin added this to the sage-7.3 milestone Jun 14, 2016
@kevinywlui
Copy link
Mannequin Author

kevinywlui mannequin commented Jun 14, 2016

@kevinywlui
Copy link
Mannequin Author

kevinywlui mannequin commented Jun 14, 2016

Commit: 6026e56

@kevinywlui
Copy link
Mannequin Author

kevinywlui mannequin commented Jun 14, 2016

New commits:

6026e56parse_label now returns index

@kevinywlui kevinywlui mannequin added the s: needs review label Jun 14, 2016
@williamstein
Copy link
Contributor

comment:3

You need to add a doctest illustrating the case that your change addresses.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 14, 2016

Changed commit from 6026e56 to 80166b8

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 14, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

80166b8added docstring, removed blank line at end of file

@kevinywlui
Copy link
Mannequin Author

kevinywlui mannequin commented Jun 14, 2016

comment:5

Replying to @williamstein:

You need to add a doctest illustrating the case that your change addresses.

Fixed.

@kevinywlui kevinywlui mannequin added s: needs review and removed s: needs work labels Jun 14, 2016
@williamstein
Copy link
Contributor

comment:7

Your docstring is not formatted properly and won't convert correctly to html/sphinx/etc. Please see
http://doc.sagemath.org/html/en/developer/coding_basics.html#template
and especially the double colons.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 14, 2016

Changed commit from 80166b8 to 79e11ab

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 14, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

79e11abfixed docstring

@kevinywlui
Copy link
Mannequin Author

kevinywlui mannequin commented Jun 14, 2016

comment:9

Replying to @williamstein:

Your docstring is not formatted properly and won't convert correctly to html/sphinx/etc. Please see
http://doc.sagemath.org/html/en/developer/coding_basics.html#template
and especially the double colons.

Fixed.

@kevinywlui kevinywlui mannequin added s: needs review and removed s: needs work labels Jun 14, 2016
@fchapoton
Copy link
Contributor

comment:11

still not correctly formatted. Lines ending with :: should be followed by an empty line

Please add you real name in the Authors box (here, top of this web page)

@kevinywlui
Copy link
Mannequin Author

kevinywlui mannequin commented Jun 14, 2016

Author: Kevin Lui

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 14, 2016

Changed commit from 79e11ab to b00abe1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 14, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

b00abe1more docstring fixes

@kevinywlui
Copy link
Mannequin Author

kevinywlui mannequin commented Jun 14, 2016

comment:14

Replying to @fchapoton:

still not correctly formatted. Lines ending with :: should be followed by an empty line

Please add you real name in the Authors box (here, top of this web page)

Hopefully this time! What's the name for this style of formatting?

@williamstein
Copy link
Contributor

comment:15

Hopefully this time! What's the name for this style of formatting?

Sphinx

@williamstein
Copy link
Contributor

Reviewer: William Stein, chapoton

@williamstein
Copy link
Contributor

comment:17

Looks good to me now.

@fchapoton
Copy link
Contributor

Changed reviewer from William Stein, chapoton to William Stein, Frédéric Chapoton

@vbraun
Copy link
Member

vbraun commented Jun 15, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants