-
-
Notifications
You must be signed in to change notification settings - Fork 482
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
Removed BUG
method and include a docstring for matrix_of_frobenius()
#37615
Removed BUG
method and include a docstring for matrix_of_frobenius()
#37615
Conversation
@kwankyu maybe you'll have time to review this? One question have is why is this method defined for curves over the rationals if we explicitly change the ring anyway? Feels like maybe I should move it to generic? |
Because only rational elements are coercable to p-adic numbers, probably. sage: x = var("x")
....: K.<a> = NumberField(x^3 + 2)
....: R.<x> = K[]
....: H = HyperellipticCurve(x^5 + a*x + 3)
....: monsky_washnitzer.matrix_of_frobenius_hyperelliptic(H.change_ring(Qp(7)))
# TypeError: Unable to coerce a to a rational |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General comments.
Fun fact: The bug is from this line. No further comment.
src/sage/schemes/hyperelliptic_curves/hyperelliptic_rational_field.py
Outdated
Show resolved
Hide resolved
src/sage/schemes/hyperelliptic_curves/hyperelliptic_rational_field.py
Outdated
Show resolved
Hide resolved
Co-authored-by: grhkm21 <[email protected]>
If CI passes then it looks good to me |
CI failure from some other unrelated bug i think |
Thanks for helping with the labels and confirming about the failure. |
Documentation preview for this PR (built with commit 4faa6a8; changes) is ready! 🎉 |
When reviewing code for hyperelliptic curves I found a method labeled bug with a typo in the name which was unnecessary. I have removed this method and included doctests which were previously absent.
📝 Checklist