Opened 3 months ago

Closed 8 weeks ago

Last modified 7 weeks 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) Commit: 613b8f6836c8d85a294478ae84fcca4f5dec68f9
Dependencies: Stopgaps:

Description

This ticket fixes all the doctests in multi_polynomial_ideal and toy_buchberger.

Change History (7)

comment:1 Changed 3 months 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 2 months 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 2 months 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 2 months 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 2 months 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 8 weeks 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 7 weeks 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.