Skip to content

Coerce int to ZZ for lift_x method #40772

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Coerce int to ZZ for lift_x method #40772

wants to merge 2 commits into from

Conversation

GiacomoPope
Copy link
Contributor

Currently there is a call to x.parent() which crashes for x of type int, but I think it should silently coerce to ZZ

@GiacomoPope
Coerce int to ZZ for lift_x method
570f11f 
Currently there is a call to `x.parent()` which crashes for `x` of type `int`, but I think it should silently coerce to `ZZ`
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 5, 2025 •
edited
Loading

Documentation preview for this PR (built with commit 33c5ebc; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@user202729
Copy link
Contributor

user202729 commented Sep 6, 2025 •
edited
Loading

Use py_scalar_to_element instead. We do support elliptic curves over RDF and such.

hopefully it's not slower.

(speaking of which… rg -U 'isinstance.*\<int\).*\n.*Integer' return a huge number of cases.)

@GiacomoPope
Copy link
Contributor Author

What's the benefit of this?

There's already the later lines which handle coercion between the parent of x and the ring of the curve -- this commit was to fix the issue with int especially.

Are you suggesting to change more of the coercion here?

@user202729
Copy link
Contributor

user202729 commented Sep 6, 2025 •
edited
Loading 

sage: E = EllipticCurve(RDF, [1, 2])
sage: E.lift_x(1.0r)
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
...
AttributeError: 'float' object has no attribute 'parent'
sage: E.lift_x(RDF(1.0))
(1.0 : -2.0 : 1.0)

@GiacomoPope
Copy link
Contributor Author

Oh thanks. I understand now

vbraun pushed a commit to vbraun/sage that referenced this pull request Sep 7, 2025
sagemathgh-40772: Coerce int to ZZ for lift_x method
b9c8945 
Currently there is a call to `x.parent()` which crashes for `x` of type
`int`, but I think it should silently coerce to `ZZ`

URL: sagemath#40772
Reported by: Giacomo Pope
Reviewer(s):
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
c: algebraic geometry p: trivial / 5 s: positive review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@GiacomoPope @user202729 @vneiger