Opened 2 years ago

Closed 2 years ago

#27674 closed enhancement (fixed)

py3: some fixes to rings/polynomial

Reported by: jhpalmieri Owned by:
Priority: major Milestone: sage-8.8
Component: python3 Keywords:
Cc: Merged in:
Authors: John Palmieri Reviewers: Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: 6cde67c (Commits, GitHub, GitLab) Commit: 6cde67c1ed35a95cc5a79415c357cd67ef636163
Dependencies: Stopgaps:

Status badges

Description (last modified by jhpalmieri)

Fixes to doctests in multi_polynomial.pyx (full), pbori.pyx (full), and binary_form_reduce.py (still one failing test).

Change History (11)

comment:1 Changed 2 years ago by jhpalmieri

  • Branch set to u/jhpalmieri/polynomial

comment:2 Changed 2 years ago by git

  • Commit set to 60bde0255afdc68cf6f736b2d5af253b55cb26e0

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​60bde02Py3 fixes to multi_polynomial.pyx, binary_form_reduce.py, pbori.pyx doctests

comment:3 Changed 2 years ago by jhpalmieri

  • Description modified (diff)

comment:4 Changed 2 years ago by jhpalmieri

  • Status changed from new to needs_review

comment:5 Changed 2 years ago by jhpalmieri

A few explanations: there was some numerical noise. Also, one doctest in multi_polynomial.pyx (list(ff)) relies on a specific ordering in a dictionary, so it seems easiest to mark the test as "random". The test 

[P._random_monomial_dfirst(3, (0,1,2)) for _ in range(10)] # py3 random

in pbori.pyx truly does seem random: I get a different answer each time I try it (start a new Sage session and run the command: I get different answers each time).

comment:6 Changed 2 years ago by chapoton

there was #27666 for pbori, but your fix looks better

comment:7 Changed 2 years ago by jhpalmieri

Sorry about the duplicated effort.

comment:8 Changed 2 years ago by git

  • Commit changed from 60bde0255afdc68cf6f736b2d5af253b55cb26e0 to 6cde67c1ed35a95cc5a79415c357cd67ef636163

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

​6cde67ctrac 27674: minor spacing cleanup

comment:9 Changed 2 years ago by jhpalmieri

I incorporated the spacing cleanup you had done at #27666 (changing m*=M.gen(j) to m *= M.gen(j), for example).

comment:10 Changed 2 years ago by chapoton

  • Reviewers set to Frédéric Chapoton
  • Status changed from needs_review to positive_review

ok, let it be, Thx

comment:11 Changed 2 years ago by vbraun

  • Branch changed from u/jhpalmieri/polynomial to 6cde67c1ed35a95cc5a79415c357cd67ef636163
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.