Opened 11 years ago

Closed 11 years ago

#9114 closed defect (fixed)

Improve documentation of infinite polynomial rings

Reported by: SimonKing Owned by: Simon King
Priority: major Milestone: sage-4.5.2
Component: commutative algebra Keywords: documentation, infinite polyonomial ring, symmetric reduction
Cc: Merged in: sage-4.5.2.alpha0
Authors: Simon King Reviewers: David Loeffler
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

At #9108, it was reported that the doc tests for symmetric ideals time out on some machines. As a quick solution, it was suggested to simply mark them as 'long'.

Here, I replace the offensive test (it is only one) by something more easy, that is still instructive.

At this occasion, I tried to improve other aspects of the doc strings as well, e.g., I tried to shorten the lines and to adhere to the standards in describing optional arguments.

The attached patch is relative to #9108, which already has a positive review. The new patch replaces the doc test that was marked 'long' in #9108.

Attachments (2)

9114_doc_infinite_polynomial.patch (87.8 KB) - added by SimonKing 11 years ago.
Shorter doc test (avoiding a time out on some systems), better doc formatting.
trac_9114-reviewer.patch (17.1 KB) - added by davidloeffler 11 years ago.
apply over previous patch

Download all attachments as: .zip

Change History (6)

Changed 11 years ago by SimonKing

Shorter doc test (avoiding a time out on some systems), better doc formatting.

comment:1 Changed 11 years ago by SimonKing

  • Milestone set to sage-4.4.3
  • Status changed from new to needs_review

Changed 11 years ago by davidloeffler

apply over previous patch

comment:2 follow-up: Changed 11 years ago by davidloeffler

Looks fine to me; it builds and passes tests under 4.4.4.alpha0, the tests pass in a reasonable length of time (25 seconds on my machine, as opposed to 17 seconds without "-long" and a ridiculously long time with "-long"). Documentation builds OK and looks good.

There is one minor problem: quite a few doctests are marked with "# indirect doc test" (with space), while the coverage script looks for "# indirect doctest". I have fixed these and added a few more doctests. (I also streamlined the __contains__ methods slightly, since all they did was try to convert x into self and then test equality, which the coercion framework does automatically anyway.) All four files relating to infinite polynomial rings now pass sage -coverage with no complaints.

Simon: if you are happy with the changes in my reviewer patch, then feel free to put the status to "positive review".

comment:3 in reply to: ↑ 2 Changed 11 years ago by SimonKing

  • Reviewers set to David Loeffler
  • Status changed from needs_review to positive_review

Replying to davidloeffler:

...

Simon: if you are happy with the changes in my reviewer patch, then feel free to put the status to "positive review".

I am happy with it! So, I changed that status to "positive review", and inserted your name as reviewer.

Best regards, Simon

comment:4 Changed 11 years ago by mpatel

  • Merged in set to sage-4.5.2.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.