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

Fix coefficient method for modular forms #32553

Closed
DavidAyotte opened this issue Sep 22, 2021 · 19 comments
Closed

Fix coefficient method for modular forms #32553

DavidAyotte opened this issue Sep 22, 2021 · 19 comments

Comments

@DavidAyotte
Copy link
Member

Currently, the coefficient method appear in the TAB-completion for a modular form, but an error occur when one tries to use it.

sage: f = ModularForms(1, 12).0
sage: f.coefficient # hit TAB
                    f.coefficient
                    f.coefficients
sage: f.coefficient(10)
Traceback (most recent call last):
...
AttributeError: 'Sequence_generic' object has no attribute 'keys'

Component: modular forms

Keywords: modular forms coefficient

Author: David Ayotte

Branch/Commit: 0071130

Reviewer: Vincent Delecroix

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

@DavidAyotte
Copy link
Member Author

@DavidAyotte
Copy link
Member Author

New commits:

a4242a6added coefficient method in sage/modular/modform/element.py

@DavidAyotte
Copy link
Member Author

Author: David Ayotte

@DavidAyotte
Copy link
Member Author

Commit: a4242a6

@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 18, 2021

comment:3

Stalled in needs_review or needs_info; likely won't make it into Sage 9.5.

@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@videlec
Copy link
Contributor

videlec commented Dec 29, 2021

Reviewer: Vincent Delecroix

@videlec
Copy link
Contributor

videlec commented Dec 29, 2021

comment:4

The conversion int(n) is not ideal since n=3/2 would be converted to 1.

@DavidAyotte
Copy link
Member Author

comment:5

Hello Vincent, thank you very much for reviewing this!

I've tried the code with your example and, strangely, I got:

sage: f = ModularForms(1, 12).0
sage: f.coefficient(3/2)
Traceback (most recent call last):
...
TypeError: no conversion of this rational to integer

I have no idea why this happens, because you are indeed right, in the command line interface we do have:

sage: int(3/2)
1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 29, 2021

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

6797e1dfix integer type conversion

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 29, 2021

Changed commit from a4242a6 to 6797e1d

@videlec
Copy link
Contributor

videlec commented Dec 30, 2021

comment:8

The error comes from

sage: f.q_expansion(3/2)
Traceback (most recent call last):
...
TypeError: no conversion of this rational to integer

@videlec
Copy link
Contributor

videlec commented Dec 30, 2021

comment:9

I would rather do

n = Integer(n)
return self.q_expansion(n+1)[n]

so that the error message will be clearer, ie pointing to the line n = ZZ(n).

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 4, 2022

Changed commit from 6797e1d to 0071130

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 4, 2022

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

0071130minor changes for clearer error message

@DavidAyotte
Copy link
Member Author

comment:11

Replying to @videlec:

I would rather do

n = Integer(n)
return self.q_expansion(n+1)[n]

so that the error message will be clearer, ie pointing to the line n = ZZ(n).

Thank you for the suggestion! Happy new year!

@videlec
Copy link
Contributor

videlec commented Jan 5, 2022

comment:12

Happy new year!

@DavidAyotte
Copy link
Member Author

comment:13

I might be wrong, but the failed doctest don't seem related to this ticket, see #32782 and #33026.

@videlec
Copy link
Contributor

videlec commented Jan 5, 2022

comment:14

Sorry. I wanted to set it to positive review the first time. Wrong manipulation.

@vbraun
Copy link
Member

vbraun commented Jan 31, 2022

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

4 participants