#16766 closed enhancement (fixed)
Improve the doc of combinat/designs/
Reported by:  ncohen  Owned by:  

Priority:  major  Milestone:  sage6.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 6 years ago by
 Branch set to u/ncohen/16766
 Commit set to 49ae09d68c23ecf4f94332187d2560144bcb6f50
comment:2 Changed 6 years ago by
 Commit changed from 49ae09d68c23ecf4f94332187d2560144bcb6f50 to d626290b67e3d14ceafa817b6c8d0388f232157e
Branch pushed to git repo; I updated commit sha1. New commits:
d626290  trac #16766: we don t want designs.deprecated_function_alias

comment:3 Changed 6 years ago by
 Status changed from new to needs_review
comment:4 Changed 6 years ago by
 Commit changed from d626290b67e3d14ceafa817b6c8d0388f232157e to 8f9ee773df53f5d60a2f4400b0aabacaf7a78975
Branch pushed to git repo; I updated commit sha1. New commits:
8f9ee77  trac #16766: Broken doctests

comment:5 Changed 6 years ago by
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/sage6.3.beta7/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 480, in _run self.execute(example, compiled, test.globs) File "/home/scratch/dimpase/sage/sage6.3.beta7/local/lib/python2.7/sitepackages/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/sage6.3.beta7/local/lib/python2.7/sitepackages/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/sage6.3.beta7/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 480, in _run self.execute(example, compiled, test.globs) File "/home/scratch/dimpase/sage/sage6.3.beta7/local/lib/python2.7/sitepackages/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 6 years ago by
comment:7 Changed 6 years ago by
 Commit changed from 8f9ee773df53f5d60a2f4400b0aabacaf7a78975 to 47cebb3a009164cffd6dd84ee62c6e0544c02147
Branch pushed to git repo; I updated commit sha1. New commits:
47cebb3  trac #16766: Broken doctest

comment:8 followup: ↓ 9 Changed 6 years ago by
Always use Return ...
instead of Returns ...
comment:9 in reply to: ↑ 8 Changed 6 years ago by
Always use
Return ...
instead ofReturns ...
HMmmmmm :P
~/sage/combinat/designs$ grep "returns " . R  wc l 172
comment:10 followup: ↓ 11 Changed 6 years ago by
I'm talking about the start of the docstring title (upper case), see http://legacy.python.org/dev/peps/pep0257/#onelinedocstrings
comment:11 in reply to: ↑ 10 Changed 6 years ago by
Replying to vbraun:
I'm talking about the start of the docstring title (upper case), see http://legacy.python.org/dev/peps/pep0257/#onelinedocstrings
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 6 years ago by
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 lowercase and with "s" because of grammar.
comment:13 Changed 6 years ago by
 Commit changed from 47cebb3a009164cffd6dd84ee62c6e0544c02147 to 6cda4b63625f50133ceedfcd6709933f013ddfda
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

comment:14 Changed 6 years ago by
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 6 years ago by
 Commit changed from 6cda4b63625f50133ceedfcd6709933f013ddfda to 06e330be39aecb4b9177c2260eaee16d13eb591d
Branch pushed to git repo; I updated commit sha1. New commits:
06e330b  trac #16766: form > from

comment:16 followup: ↓ 17 Changed 6 years ago by
 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 6 years ago by
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 6 years ago by
 Branch changed from u/ncohen/16766 to 06e330be39aecb4b9177c2260eaee16d13eb591d
 Resolution set to fixed
 Status changed from positive_review to closed
comment:19 Changed 6 years ago by
 Commit 06e330be39aecb4b9177c2260eaee16d13eb591d deleted
Thaaaaaaaaaaankks !
New commits:
trac #16766: Improve the doc of combinat/designs/