#18439: is_projective_plane for incidence structure
-------------------------------------+-------------------------------------
Reporter: q.honore | Owner:
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-6.7
Component: combinatorial | Resolution:
designs | Merged in:
Keywords: | Reviewers: Vincent Delecroix
Authors: Quentin Honoré | Work issues:
Report Upstream: N/A | Commit:
Branch: | 0824ca649d08566381eb74d525744cf3c4aafeb4
u/q.honore/is_projective_plane | Stopgaps:
Dependencies: |
-------------------------------------+-------------------------------------
Changes (by vdelecroix):
* status: needs_review => needs_work
* reviewer: => Vincent Delecroix
Comment:
Hi Quentin,
1. there is no need to mention the first argument {{{``s``}}} in the
`INPUT` section. Moreover, as you can see this argument is often denoted
`self` in other methods. That way it is clear that it is not really an
argument.
2. In the verbose output
{{{
The number of points must be k^2 + k + 1 =6, got 7
}}}
it would be better to also have a space after the k^2 + k + 1, in other
words
{{{
The number of points must be k^2 + k + 1 = 6, got 7
}}}
3. Why did you indent your comments (i.e. the lines starting with `#`)? If
you look at other methods like `__contains__` or `degree` you will see
that there is no indentation.
4. I think you would better move your method down in the file. At least
below the comment at line 1279
{{{
#####################
# real computations #
#####################
}}}
As you can see, the file `incidence_structures.py` is rather long and it
is important to keep it organized as much as possible.
5. It would be nice to add an example with labels which are not integers
{{{
sage: B = ['abc', 'adf', 'age', 'bde', 'bgf', 'cgd', 'cef']
sage: IncidenceStructure(B).is_projective_plane()
True
sage: B[-1] = 'cgf'
sage: B[-2] = 'ced'
sage: IncidenceStructure(B).is_projective_plane(verbose=True)
More than one line through d and e
False
}}}
that way you test that your error message explicitely mentions the
points (and not their internal indices).
6. I am not exactly sure but it might be that the method `is_t_design`
already implements this. The parameters would be `v=k^2+k+1`, `t=2` and
`lambda=1`.
Vincent
--
Ticket URL: <http://trac.sagemath.org/ticket/18439#comment:2>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
