#17898: Removal of wrong stopgap
-------------------------------------+-------------------------------------
Reporter: aschilling | Owner:
Type: defect | Status: needs_work
Priority: major | Milestone: sage-6.6
Component: combinatorics | Resolution:
Keywords: stopgap, | Merged in:
partitions | Reviewers: Travis Scrimshaw,
Authors: Travis Scrimshaw, | Anne Schilling
Anne Schilling | Work issues:
Report Upstream: N/A | Commit:
Branch: | d3de7cf960cb38e03d69d3e4b8951bcc9ddd830a
public/combinat/fix_bad_stopgap-17898| Stopgaps:
Dependencies: |
-------------------------------------+-------------------------------------
Comment (by jhpalmieri):
I hate to get involved in this because the level of discourse is not up to
Sage's usual standards.
Replying to [comment:43 tscrim]:
> The first one is documented:
This is not actually clear to me. I think by "the first one" you mean
`Compositions(6, max_part=3, max_slope=-1).list()`, but the note could be
viewed as just discussing inner and outer partitions which have not been
explicitly specified. The note is not very prominent, in any case, and
other similar cases have no such accompanying warning in the
documentation:
{{{
sage: Compositions(6, max_part=3, max_slope=-1).list() # "documented"
[[3, 3], [3, 2, 1]]
sage: list(IntegerVectors(6, max_part=3, max_slope=-1, min_part=1)) # not
documented: a bug
[[3, 3], [3, 2, 1]]
}}}
Is `length` a valid option for `IntegerVectors`?
{{{
sage: list(IntegerVectors(4, length=3, min_part=1)) # a bug?
[[4], [3, 1], [2, 2], [2, 1, 1], [1, 3], [1, 2, 1], [1, 1, 2], [1, 1, 1,
1]
}}}
And as an aside, something for a separate ticket, there is
{{{
sage: list(IntegerVectors(6, max_part=3, max_slope=-1, min_part=1))
[[3, 3], [3, 2, 1]]
sage: IntegerVectors(6, max_part=3, max_slope=-1, min_part=1).list()
---------------------------------------------------------------------------
NotImplementedError Traceback (most recent call
last)
<ipython-input-50-a7a7eaaa2968> in <module>()
----> 1 IntegerVectors(Integer(6), max_part=Integer(3),
max_slope=-Integer(1), min_part=Integer(1)).list()
/Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/python2.7/site-
packages/sage/combinat/integer_vector.pyc in list(self)
1126 """
1127 if 'max_length' not in self.constraints:
-> 1128 raise NotImplementedError("infinite list") # no list
from infinite iter
1129 else:
1130 return list(self)
NotImplementedError: infinite list
}}}
Why does `list(...)` work differently from `(...).list`?
Anyway, the fact that the documentation for `IntegerVectors` lists none of
the options is not acceptable. The docstring includes the definition
`IntegerVectors(n=None, k=None, **kwargs)`, and not only does it list none
of the keyword arguments, it doesn't even say what `n` and `k` mean. As
has already been pointed out, the documentation for `IntegerListsLex`
doesn't list `max_part` in its possible inputs. The documentation for
`Compositions` doesn't list any of the inputs. It does say `See also
"Composition", "Partitions", "IntegerVectors"`, the documentation of one
of which discusses the options. (It should probably say `See "Partitions"
for a description of the available options`, or something like that.) The
state of the documentation is terrible, and it seems clear that whoever
wrote and reviewed this code was not thinking about how anyone would
actually use it.
(I also think that any docstring that discusses slope should define it the
way the docstring for `IntegerListsLex` does (explicitly saying
`l[i+1]-l[i]`) rather than the way the docstring for `Partitions` does
(`the difference between successive parts`, because that could mean
`l[i+1]-l[i]` or its negative or possibly its absolute value). This is
made more confusing by an example in the `Partitions` documentation about
a partition whose "parts differ by at least 2", which sounds like
`min_slope=2`, but is actually `max_slope=-2`.)
From this point of view, the idea of a stopgap is pretty mild: we could
actually be discussing ripping all of this code out of Sage until its
documentation, and probably its input checking, is in good shape. We
shouldn't rip it out, of course, but this part of Sage definitely needs
work.
--
Ticket URL: <http://trac.sagemath.org/ticket/17898#comment:48>
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.
