Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#16766 closed enhancement (fixed)

Improve the doc of combinat/designs/

Reported by: ncohen Owned by:
Priority: major Milestone: sage-6.3
Component: combinatorial designs Keywords:
Cc: vdelecroix, knsam, dimpase, brett Merged in:
Authors: Nathann Cohen Reviewers: Volker Braun, Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: 06e330b (Commits) Commit:
Dependencies: Stopgaps:

Description

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

Change History (19)

comment:1 Changed 5 years ago by ncohen

  • Branch set to u/ncohen/16766
  • Commit set to 49ae09d68c23ecf4f94332187d2560144bcb6f50

New commits:

49ae09dtrac #16766: Improve the doc of combinat/designs/

comment:2 Changed 5 years ago by git

  • Commit changed from 49ae09d68c23ecf4f94332187d2560144bcb6f50 to d626290b67e3d14ceafa817b6c8d0388f232157e

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

d626290trac #16766: we don t want designs.deprecated_function_alias

comment:3 Changed 5 years ago by ncohen

  • Status changed from new to needs_review

comment:4 Changed 5 years ago by git

  • Commit changed from d626290b67e3d14ceafa817b6c8d0388f232157e to 8f9ee773df53f5d60a2f4400b0aabacaf7a78975

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

8f9ee77trac #16766: Broken doctests

comment:5 Changed 5 years ago by dimpase

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

comment:6 Changed 5 years ago by ncohen

Oops. Sorry. It was broken by #16597.

Fixed !

Nathann

comment:7 Changed 5 years ago by git

  • Commit changed from 8f9ee773df53f5d60a2f4400b0aabacaf7a78975 to 47cebb3a009164cffd6dd84ee62c6e0544c02147

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

47cebb3trac #16766: Broken doctest

comment:8 follow-up: Changed 5 years ago by vbraun

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

comment:9 in reply to: ↑ 8 Changed 5 years ago by ncohen

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

HMmmmmm :-P

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

comment:10 follow-up: Changed 5 years ago by 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

comment:11 in reply to: ↑ 10 Changed 5 years ago by dimpase

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.

comment:12 Changed 5 years ago by vbraun

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.

comment:13 Changed 5 years ago by git

  • Commit changed from 47cebb3a009164cffd6dd84ee62c6e0544c02147 to 6cda4b63625f50133ceedfcd6709933f013ddfda

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

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

comment:14 Changed 5 years ago by dimpase

more typos: in several places you have

Obtained form the La Jolla Covering Repository

which should be

Obtained from the La Jolla Covering Repository

comment:15 Changed 5 years ago by git

  • Commit changed from 6cda4b63625f50133ceedfcd6709933f013ddfda to 06e330be39aecb4b9177c2260eaee16d13eb591d

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

06e330btrac #16766: form -> from

comment:16 follow-up: Changed 5 years ago by dimpase

  • Reviewers set to Volker Braun, Dima Pasechnik
  • Status changed from needs_review to positive_review

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.

comment:17 in reply to: ↑ 16 Changed 5 years ago by ncohen

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

comment:18 Changed 5 years ago by vbraun

  • Branch changed from u/ncohen/16766 to 06e330be39aecb4b9177c2260eaee16d13eb591d
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:19 Changed 5 years ago by ncohen

  • Commit 06e330be39aecb4b9177c2260eaee16d13eb591d deleted

Thaaaaaaaaaaankks !

Note: See TracTickets for help on using tickets.