Opened 4 months ago

Closed 4 months 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) Commit: 190efb93880e76d97cea2b38c9e48daff8c5bb47
Dependencies: Stopgaps:

Description


Change History (8)

comment:1 Changed 4 months 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 4 months 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 4 months 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 4 months 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 4 months 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 4 months ago by chapoton

  • Status changed from needs_work to needs_review

fixed the pyflakes warning

comment:7 Changed 4 months ago by roed

  • Status changed from needs_review to positive_review

Sounds good.

comment:8 Changed 4 months 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.