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.

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

add .torsion_basis() method to EllipticCurve_finite_field #34982

Merged
merged 1 commit into from
Feb 19, 2023
Merged

add .torsion_basis() method to EllipticCurve_finite_field #34982

merged 1 commit into from
Feb 19, 2023

Conversation

yyyyx4
Copy link
Member

@yyyyx4 yyyyx4 commented Feb 6, 2023

Fixes #34718.

@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (develop@9116c55). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files 
@@            Coverage Diff             @@
##             develop   #34982   +/-   ##
==========================================
  Coverage           ?   88.59%           
==========================================
  Files              ?     2136           
  Lines              ?   396146           
  Branches           ?        0           
==========================================
  Hits               ?   350952           
  Misses             ?    45194           
  Partials           ?        0

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@roed314
Copy link
Contributor

roed314 commented Feb 7, 2023

There are failing tests, but otherwise this looks good to me.

@yyyyx4 yyyyx4 force-pushed the public/elliptic_curve_torsion_basis branch from c9badc3 to efb609f Compare February 8, 2023 02:42
@yyyyx4
Copy link
Member Author

yyyyx4 commented Feb 8, 2023

The test failure is unrelated to the changes here. See #35017.

@yyyyx4 yyyyx4 force-pushed the public/elliptic_curve_torsion_basis branch from efb609f to 110d9bf Compare February 8, 2023 11:22
@yyyyx4 yyyyx4 requested a review from roed314 February 9, 2023 12:30
@JohnCremona
Copy link
Member

Looks good to me and tests do now pass -- @roed314 can you do a positive review and change the label accordingly? (I can do the latter, but you are the official reviewer.)

roed314
roed314 approved these changes Feb 10, 2023
View reviewed changes
Copy link
Contributor

@roed314 roed314 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@mkoeppe mkoeppe mentioned this pull request Feb 12, 2023
Closed
@vbraun vbraun merged commit da48f14 into sagemath:develop Feb 19, 2023
@mkoeppe mkoeppe added this to the sage-10.0 milestone Feb 19, 2023
@yyyyx4 yyyyx4 deleted the public/elliptic_curve_torsion_basis branch February 20, 2023 03:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roed314 roed314 roed314 approved these changes

Assignees
No one assigned
Labels
c: elliptic curves p: minor / 4 t: enhancement
Projects
None yet
Milestone
sage-10.0
Development

Successfully merging this pull request may close these issues.

add .torsion_basis() method to EllipticCurve_finite_field
8 participants
@yyyyx4 @codecov-commenter @roed314 @JohnCremona @vbraun @kwankyu @fchapoton @mkoeppe