Opened 3 years ago

Closed 3 years ago

#26570 closed enhancement (fixed)

py3: fix some doctests in elliptic curves

Reported by: chapoton Owned by:
Priority: major Milestone: sage-8.5
Component: python3 Keywords:
Cc: cremona Merged in:
Authors: Frédéric Chapoton Reviewers: David Roe
Report Upstream: N/A Work issues:
Branch: 190efb9 (Commits, GitHub, GitLab) Commit: 190efb93880e76d97cea2b38c9e48daff8c5bb47
Dependencies: Stopgaps:

Status badges

Description


Change History (8)

comment:1 Changed 3 years ago by chapoton

  • Branch set to u/chapoton/26570
  • Commit set to d4f5eb4fdf1935175ec7beab24f0f8d332ca2023
  • Status changed from new to needs_review

New commits:

​d4f5eb4py3: some fixes in ellilptic curves

comment:2 Changed 3 years ago by roed

  • Reviewers set to David Roe
  • Status changed from needs_review to positive_review

These look fine to me, but I'm curious why the changes are necessary. Aside from whitespace, it looks like you're changing sqrt(blah) to blah.sqrt() and round(blah) to blah.round(). Is there a reason the former aren't okay in python 3?

comment:3 Changed 3 years ago by roed

  • Status changed from positive_review to needs_work

Pyflakes is complaining about sqrt not being used: 

src/sage/schemes/elliptic_curves/padic_lseries.py:727: 'sage.functions.all.sqrt' imported but unused

comment:4 Changed 3 years ago by chapoton

No special reason for sqrt.

In python3, the function round(x) calls the method x.round() that sage does not have. Sage only has x.round() with a slightly different semantic.

comment:5 Changed 3 years ago by git

  • Commit changed from d4f5eb4fdf1935175ec7beab24f0f8d332ca2023 to 190efb93880e76d97cea2b38c9e48daff8c5bb47

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

​190efb9trac 26570 fix import (pyflakes)

comment:6 Changed 3 years ago by chapoton

  • Status changed from needs_work to needs_review

fixed the pyflakes warning

comment:7 Changed 3 years ago by roed

  • Status changed from needs_review to positive_review

Sounds good.

comment:8 Changed 3 years ago by vbraun

  • Branch changed from u/chapoton/26570 to 190efb93880e76d97cea2b38c9e48daff8c5bb47
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.