#17979: Reimplementation of IntegerListsLex
-------------------------------------+-------------------------------------
Reporter: aschilling | Owner:
Type: defect | Status: needs_work
Priority: blocker | Milestone: sage-6.6
Component: combinatorics | Resolution:
Keywords: days64 | Merged in:
Authors: Bryan Gillespie, | Reviewers:
Anne Schilling, Nicolas M. Thiery | Work issues: support n in an
Report Upstream: N/A | iterable
Branch: | Commit:
public/ticket/17979 | 6153cf8d39dc23cb41a3d0c2ea66ba5dc3abd2c0
Dependencies: | Stopgaps:
-------------------------------------+-------------------------------------
Comment (by ncohen):
Helloooooooooooo,
Here are some comments (mostly doc) about the current branch
- {{{
An integer list is a list l of nonnegative integers, its parts. The
length
of l is the number of its parts
Note Two valid integer lists are considered equivalent if they only
differ by
trailing zeroes.
}}}
Unless the length of a list is the number of its nonzero entries, it
does not
seem to be properly defined.
- {{{The constraints on the lists are as follows:}}} -- in what follows
you use
a variable `l` often (probably one of the lists): could you say so
explicitly?
- It seems that currently the method accepts input that does not satisfy
the
constraints that you list, i.e.:
{{{
sage: IntegerListsLex(min_n=4)
Integer lists of sum between 4 and 0 satisfying certain constraints
sage: list(IntegerListsLex(min_n=4))
[]
}}}
Should they really be considered as 'constraints', if the code accepts
them
and returns sensible output (i.e. empty sets)? When I read those lines,
I
expected the code to raise a `ValueError` exception on them.
- {{{Lower and upper bounds}}}: the text about constant values for
floor/ceiling
belongs to the INPUT block.
- `waiver` -- the description of `waiver` in the INPUT block is very
mysterious. If it is only meant for internal purposes, could you say so
in its
description?
- {{{Next we obtain all lists of sum 4 and length 4 such that l[i] <=
i:}}} --
missing backticks at the end.
- Comparative timings: should they really appear in the function's
documentation? To me the trac ticket is the right place for that.
- There are two 'TESTS' sections
- {{{self.warning = False # warning for dangerous (but possibly valid)
usage}}}
-- could say what this flag does?
- the INPUT blocks says that `n` can be a list. Could you add there an
explanation of what it means?
- {{{
elif min_part == -infinity:
raise(ValueError("min_part can't be negative or infinite"))
}}}
Did you want to test `min_part<0`? If so, could you add a doctest for
that?
- About the message
{{{warn("""When the user specifies a method, then (s)he is responsible
that the algorithm
will not hang. Also note that the specified function should
start at 0 rather than 1.
Before trac#17979 the indexing was ambiguous and sometimes
started at 1.""")}}}
From time to time we receive bug reports on sage-devel or sage-support
in
which the users beg us to forgive them in case what they think might be
a bug
could actually be their mistake. Could this message be changed to
something
more `technical`? Something like `you defined ceiling=[...] to be a
function,
and we cannot swear that this call will not hang`?
- About the copyright header: I never saw any patch remove the name of
other
persons in a copyright header. Don't know what the policy is.
{{{
#!diff
-# Copyright (C) 2007 Mike Hansen <[email protected]>,
-# Copyright (C) 2012 Travis Scrimshaw <[email protected]>
+# Copyright (C) 2015 Bryan Gillespie <[email protected]>,
}}}
Thanks,
Nathann
--
Ticket URL: <http://trac.sagemath.org/ticket/17979#comment:27>
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.
