#16534: Basic Block design methods
-------------------------------------+-------------------------------------
Reporter: brett | Owner:
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-6.4
Component: combinatorial | Resolution:
designs | Merged in:
Keywords: Block Design, | Reviewers:
Incidence Structure, Residual, | Work issues:
Derived, Complement, Supplement, | Commit:
Point Deletion | f72a8ed01c5745f16f8f1bb317cd5e81d6cf6584
Authors: Brett Stevens | Stopgaps:
Report Upstream: N/A |
Branch: u/brett/design |
Dependencies: #16553 |
-------------------------------------+-------------------------------------
Changes (by ncohen):
* status: needs_review => needs_work
Comment:
Hellooooooo Brett,
Here is a first-pass review of your branch:
- About giving credit to the authors: I believe that this part of your
branch is
a bit too verbose, if I may say. I am not saying that you should not
leave a
trace of what you do in the source code, but on the other hand we are
many to
work on the same files, and we patch them very often. We cannot leave a
'log'
of what is being done to the files every time, for if we did most of the
files
would be credit to the authors.
To give you an idea: after all the work we did recently in the design
code
(around 60+ tickets), my name and Vincent's appear for a total of three
times
in the whole designs/ folder.
Furthermore, we used to advise developers to add their name in the
functions
that they implement, just in case we need to find out who wrote them
later on:
it turns out that the 'git blame' command does that 1000x more easily,
so
this is not needed anymore. Added to that, it is often true that some
function
initially written by X is then totally rewritten by Y... and only the
name of
X appears in the function's doc.
http://git-scm.com/docs/git-blame
Thus please, if you do want to leave your name somewhere, add a line
with a
short description of what you did. Vincent left the following comment at
the
head of `incidence_structure.py`:
{{{
- Vincent Delecroix (2014): major rewrite
}}}
- About line length: we try to keep lines of doc/code to a maximum of 80
characters (unless that makes the doc/code much harder to read)
- We try to write the documentation of each function following a pattern:
1) A one-line description of what the function does
2) Longer explanations if needed in the next paragraph
See
http://www.sagemath.org/doc/developer/coding_basics.html#the-docstring-
of-a-function-content
- In the specific case of `incidence_structure.py`, you will find at the
top of
the page an index of all methods, which makes it easier to browse.
Please add
yours to that list.
http://www.sagemath.org/doc/reference/graphs/sage/combinat/designs/incidence_structures.html
- Could you use backticks {{{`x`}}} instead of {{{$x$}}} in this file?
This is
the style that is used there.
- `pt in self.ground_set()` builds the list of points only to throw it
away
afterwards. You will find how to do that in the code of other functions.
- A big block of code from `derived_incidence_structure_at_point` seems to
be
equivalent to
{{{
[[x for x in B if x!=p] for B in self if p in self]
}}}
I also do not understand to what end you sort the sets.
- about `derived_incidence_structure_at_block/point` and
`residual_incidence_structure_at_block/point`. I would be for turning
these
two very similar functions to something like
`derived_incidence_structure(at_point=p)` and
`derived_incidence_structure(at_block=b)`. Actually, perhaps
`derived_structure` and `residual_structure` may be sufficient.
- about `delete_points` -- I feel a bit uneasy with respect to this
function, as
contrarily to what 'graph' does it does not remove the points 'inplace'
but
returns a copy. Furthermore, 'removing' a point x always leaves some
uncertainty with respect to the blocks containing x: are they reduced?
deleted?
- We already have `.induced_substructure` and `.trace` which more or less
do the
job. Perhaps we could add an argument to them in order to remove points
from
the whole design, instead of giving the list of those that will stay?
Tell me what you think about the points above. Cheers,
Nathann
--
Ticket URL: <http://trac.sagemath.org/ticket/16534#comment:14>
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.
