Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#27908 closed enhancement (fixed)

py3: fix doctests in multi_polynomial_ideal and toy_buchberger

Reported by: gh-mwageringel Owned by:
Priority: major Milestone: sage-8.9
Component: python3 Keywords:
Cc: Merged in:
Authors: Markus Wageringel Reviewers: Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: 613b8f6 (Commits, GitHub, GitLab) Commit: 613b8f6836c8d85a294478ae84fcca4f5dec68f9
Dependencies: Stopgaps:

Status badges


This ticket fixes all the doctests in multi_polynomial_ideal and toy_buchberger.

Change History (7)

comment:1 Changed 3 years ago by gh-mwageringel

  • Authors set to Markus Wageringel
  • Branch set to u/gh-mwageringel/27908
  • Commit set to 81924255c063cf7b27688bd9ecfdbe00cf2ddbcb
  • Status changed from new to needs_review

The hash of polynomials intenionally makes use of the string hash of its variable names. This differs in each Python 3 session, so that the order of polynomials when iterating over sets is not consistent. Sets are used both in the implementation of random_element() and buchberger(), so this is the root reason for all the doctest failures fixed in this ticket.

In toy_buchberger I chose to keep the protocol output of buchberger() by declaring it random, as it seems educationally useful. I could have used a py2 directive, but chose not to mainly to keep the test from being removed when Python 2 support is dropped. I removed the protocol output for buchberger_improved() and only test for the number of reductions-to-zero, as it seems enough to see the protocol once in the docs.

I also fixed most of the PEP warnings in toy_buchberger.

New commits:

272da97Trac #27908: py3: fix doctests in multi_polynomial_ideal
613b8f6Trac #27908: py3: fix doctests in toy_buchberger
8192425Trac #27908: some PEP cleaning of toy_buchberger

comment:2 Changed 3 years ago by chapoton

Could you please make the same branch without the pep changes ? I see that you have separated commits, and the pep changes only make it more difficult to understand what you really do. Thanks!

comment:3 Changed 3 years ago by git

  • Commit changed from 81924255c063cf7b27688bd9ecfdbe00cf2ddbcb to 613b8f6836c8d85a294478ae84fcca4f5dec68f9

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

comment:4 Changed 3 years ago by gh-mwageringel

Of course. I have removed the last commit and will create a new ticket for it, once this one is merged.

comment:5 Changed 3 years ago by chapoton

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

ok, thanks. This looks good enough. I am setting to positive.

comment:6 Changed 3 years ago by vbraun

  • Branch changed from u/gh-mwageringel/27908 to 613b8f6836c8d85a294478ae84fcca4f5dec68f9
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:7 Changed 3 years ago by embray

  • Milestone changed from sage-8.8 to sage-8.9

Not in Sage 8.8. Let's please to try keep tickets' milestones related to the release in which we actually intend to include them, and in particular the release in which they were actually included, especially when closing tickets.

Note: See TracTickets for help on using tickets.