#17053: Add function for disjoint union and ordinal sum of posets
-------------------------------------+-------------------------------------
Reporter: jmantysalo | Owner:
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-wishlist
Component: combinatorics | Resolution:
Keywords: | Merged in:
Authors: Jori Mäntysalo | Reviewers: Nathann Cohen
Report Upstream: N/A | Work issues:
Branch: | Commit:
u/jmantysalo/add_function_for_direct_sum_of_posets|
7a801e8d3045ef87d8a35d8491ed2e76b18d30a3
Dependencies: | Stopgaps:
-------------------------------------+-------------------------------------
Changes (by {'newvalue': u'Jori M\xe4ntysalo', 'oldvalue': ''}):
* status: needs_review => needs_work
* reviewer: => Nathann Cohen
* author: => Jori Mäntysalo
Comment:
Review of your branch in its current state:
1) Lines are 100 characters long: we try to keep it 80characters max
2) It may not be visible to you with the text editor you use, but the
lines you skipped are actually filled with spaces.
http://programmers.stackexchange.com/questions/121555/why-is-trailing-
whitespace-a-big-deal
3) Please remove those `\` at the end of the lines in the documentation.
If the doc does not build properly, it is because this is incorrect
{{{
INPUT:
- a very long
line of doc
}}}
But this will work:
{{{
INPUT:
- a very long
line of doc
}}}
Sphinx (the software that builds the doc) is not very pleasant software to
work with.
3) I usually write {{{If ``labels='pairs'``}}} instead of {{{If
labels='pairs'}}} in the doc because it is actual Python code, but it is
probably up to you. Same for mathematical things like {{{`(0,x)`}}}.
4) The doc of ordinal sum claims that it returns a lattice. It does not.
5) What about an "elif" ?
{{{
else:
if labels != 'pairs':
}}}
6) please use `isinstance` instead of `type` or `in LatticePosets()`. The
latter is related to categories: it is probably safe to use by itself but
it is a bit weird to see some calls to 'type' and then category queries.
7) It is a bit weird to see that the elements labelled with 1 are smaller
than the elements labelled with 0:
{{{
sage: p1=posets.ChainPoset(1)
sage: p1.ordinal_sum(p1).is_greater_than((0,0),(1,0))
True
sage: p1.ordinal_sum(p1).is_greater_than((1,0),(0,0))
False
}}}
8) The description of "labels" is written in a slightly incorrect english
`^^;`
Nathann
--
Ticket URL: <http://trac.sagemath.org/ticket/17053#comment:22>
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.
