Improve the doc of combinat/designs/

[nathanncohen]

It seems that there will be a release very soon, and it would be a pity to show a bad doc to the world. Especially when everybody will find our new design stuff incredible ! :-P

(pleasepleasepleaseplease if you can review it fast, help meeeee !!)

Nathann

CC: @videlec @KPanComputes @dimpase @brettpim

Component: combinatorial designs

Author: Nathann Cohen

Branch: 06e330b

Reviewer: Volker Braun, Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/16766

[nathanncohen]

Commit: 49ae09d

[nathanncohen]

Branch: u/ncohen/16766

[nathanncohen]

New commits:

49ae09d

trac #16766: Improve the doc of combinat/designs/

[sagetrac-git]

Changed commit from 49ae09d to d626290

[sagetrac-git]

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

d626290

trac #16766: we don t want designs.deprecated_function_alias

[sagetrac-git]

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

8f9ee77

trac #16766: Broken doctests

[sagetrac-git]

Changed commit from d626290 to 8f9ee77

[dimpase]

comment:5

trying tests with gap_packages, getting

age -t src/sage/combinat/designs/block_design.py
**********************************************************************
File "src/sage/combinat/designs/block_design.py", line 200, in sage.combinat.designs.block_design.ProjectiveGeometryDesign
Failed example:
    BD = designs.ProjectiveGeometryDesign(2, 1, GF(2), algorithm="gap") # optional - gap_packages (design package)
Exception raised:
    Traceback (most recent call last):
      File "/home/scratch/dimpase/sage/sage-6.3.beta7/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 480, in _run
        self.execute(example, compiled, test.globs)
      File "/home/scratch/dimpase/sage/sage-6.3.beta7/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 839, in execute
        exec compiled in globs
      File "<doctest sage.combinat.designs.block_design.ProjectiveGeometryDesign[5]>", line 1, in <module>
        BD = designs.ProjectiveGeometryDesign(Integer(2), Integer(1), GF(Integer(2)), algorithm="gap") # optional - gap_packages (design package)
      File "/home/scratch/dimpase/sage/sage-6.3.beta7/local/lib/python2.7/site-packages/sage/combinat/designs/block_design.py", line 223, in ProjectiveGeometryDesign
        gap.eval("D := PGPointFlatBlockDesign( %s, %s, %d )"%(n,q,d))
    NameError: global name 'q' is not defined
**********************************************************************
File "src/sage/combinat/designs/block_design.py", line 201, in sage.combinat.designs.block_design.ProjectiveGeometryDesign
Failed example:
    BD.is_t_design(return_parameters=True)                              # optional - gap_packages (design package)
Exception raised:
    Traceback (most recent call last):
      File "/home/scratch/dimpase/sage/sage-6.3.beta7/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 480, in _run
        self.execute(example, compiled, test.globs)
      File "/home/scratch/dimpase/sage/sage-6.3.beta7/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 839, in execute
        exec compiled in globs
      File "<doctest sage.combinat.designs.block_design.ProjectiveGeometryDesign[6]>", line 1, in <module>
        BD.is_t_design(return_parameters=True)                              # optional - gap_packages (design package)
    NameError: name 'BD' is not defined

[nathanncohen]

comment:6

Oops. Sorry. It was broken by #16597.

Fixed !

Nathann

[sagetrac-git]

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

47cebb3

trac #16766: Broken doctest

[sagetrac-git]

Changed commit from 8f9ee77 to 47cebb3

[vbraun]

comment:8

Always use Return ... instead of Returns ...

[nathanncohen]

comment:9

Always use Return ... instead of Returns ...

HMmmmmm :-P

~/sage/combinat/designs$ grep "returns " . -R | wc -l
172

[vbraun]

comment:10

I'm talking about the start of the docstring title (upper case), see
http://legacy.python.org/dev/peps/pep-0257/#one-line-docstrings

[dimpase]

comment:11

Replying to @vbraun:

I'm talking about the start of the docstring title (upper case), see
http://legacy.python.org/dev/peps/pep-0257/#one-line-docstrings

Yes this seems clear.

But how about lines like:

Construction 1	two_n()	Returns a Steiner Quadruple System on 2n points

I'd say, make it either Return, or returns.

[vbraun]

comment:12

Since this is a table, I would make the third column follow the docstring conventions. Clearly Nathan intended to tabulate the docstring titles there. If it were a single sentence ("Construction 1 returns a Steiner Quadruple System") then lower-case and with "s" because of grammar.

[sagetrac-git]

Changed commit from 47cebb3 to 6cda4b6

[sagetrac-git]

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

6cda4b6

trac #16766: Git 101: How to create a conflict with 10 others patches in needs_review

[dimpase]

comment:14

more typos: in several places you have

Obtained form the La Jolla Covering Repository

which should be

Obtained from the La Jolla Covering Repository

[sagetrac-git]

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

06e330b

trac #16766: form -> from

[sagetrac-git]

Changed commit from 6cda4b6 to 06e330b

[dimpase]

comment:16

Lastly, I don't know whether that (1,2...,5) in the following

[Hanani60]	(1, 2, 3, 4, 5) Haim Hanani, On quadruple systems...

is how it should be. (Actually, indeed, there are 5 occurences of [Hanani60]_ in the text, but why this should be typeset like this by Sphinx, I have no idea.
If this can be fixed (in another ticket), it should.

Good enough for a positive review.

[dimpase]

Reviewer: Volker Braun, Dima Pasechnik

[nathanncohen]

comment:17

Yoooooooooo !!

Lastly, I don't know whether that (1,2...,5) in the following

[Hanani60]	(1, 2, 3, 4, 5) Haim Hanani, On quadruple systems...

is how it should be. (Actually, indeed, there are 5 occurences of [Hanani60]_ in the text, but why this should be typeset like this by Sphinx, I have no idea.
If this can be fixed (in another ticket), it should.

I have always seen it like that. And I don't get why either (who needs backward links ?...)

Nathann

[vbraun]

Changed branch from u/ncohen/16766 to 06e330b

[nathanncohen]

comment:19

Thaaaaaaaaaaankks !

[nathanncohen]

Changed commit from 06e330b to none