Opened 3 years ago

Closed 3 years ago

#27909 closed enhancement (fixed)

py3: fix a doctest in binary_form_reduce

Reported by: gh-mwageringel Owned by:
Priority: major Milestone: sage-8.8
Component: python3 Keywords:
Cc: bhutz Merged in:
Authors: Markus Wageringel Reviewers: Ben Hutz
Report Upstream: N/A Work issues:
Branch: 569bf72 (Commits, GitHub, GitLab) Commit: 569bf72103cc6678f4004f27d0210ceac178e437
Dependencies: Stopgaps:

Status badges


This ticket fixes a Python 3 doctest failure in smallest_poly() in sage.rings.polynomial.binary_form_reduce.

In Python 3, the result of the function differs due to tiny floating point inaccuracies. I checked that the value that is minimized inside the function is the same in Python 2 and 3 (up to small tolerance), so apparently the result of the computation is not unique. In order to stay consistent between Python versions, I replaced the doctest by a test that checks the properties of the result instead of the result itself.

What I am not sure about: Does smallest_poly() return the polynomial for which get_bound_poly() attains its minimum? If so, perhaps this could be clarified in the documentation.

Change History (5)

comment:1 Changed 3 years ago by gh-mwageringel

  • Authors set to Markus Wageringel
  • Branch set to u/gh-mwageringel/27909
  • Commit set to 6e576523aaf203e417612cb057249ff1a33d36b0
  • Status changed from new to needs_review

New commits:

6e57652Trac #27909: py3: fix doctest in smallest_poly()

comment:2 Changed 3 years ago by gh-mwageringel

  • Cc bhutz added

comment:3 Changed 3 years ago by git

  • Commit changed from 6e576523aaf203e417612cb057249ff1a33d36b0 to 569bf72103cc6678f4004f27d0210ceac178e437

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

569bf72fix pyflakes in binary_form_reduce

comment:4 Changed 3 years ago by bhutz

  • Reviewers set to Ben Hutz
  • Status changed from needs_review to positive_review

This looks fine to me. The deletions all seem to be unused items that were missed when the original code was checked in.

comment:5 Changed 3 years ago by vbraun

  • Branch changed from u/gh-mwageringel/27909 to 569bf72103cc6678f4004f27d0210ceac178e437
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.