#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: Nathann Cohen, Jeroen
Anne Schilling, Nicolas M. Thiery | Demeyer, Travis Scrimshaw
Report Upstream: N/A | Work issues:
Branch: | Commit:
public/ticket/17979 | e151d78ea32b9cac2047e2faff3247bdcce1f5d5
Dependencies: | Stopgaps:
-------------------------------------+-------------------------------------
Comment (by jdemeyer):
Replying to [comment:412 ncohen]:
> - About this paragraph:
> {{{
> The set of allowable constraints has been specifically designed to
enable
> iteration with a good time and memory complexity in most practical use
cases,
> and in inverse lexicographic order (see below)
> }}}
> ...
> This statement sounds wrong to me, and so I believe that it should be
removed.
Agreed.
> - I was surprised by the following output:
> {{{
> sage: IntegerListsLex(3,max_length=3,floor=[1,1,1]).list()
> [[3], [2, 1], [1, 2], [1, 1, 1]]
> }}}
> I woud have expected the length of the lists to be at least 3, as I
requested
> the first three parts to be `>=1`. The documentation is consistent,
but I
> still find it surprising.
What should be done to make it less surprising? It is completely
consistent with `min_part=1`.
> - ``check`` seems misnamed. It is not about checking the input or
output, only
> about displaying warnings. What about `disable_warnings` instead?
I asked for this argument to be called `check`. Usually, it's exceptions
which are controlled by a `check` argument, but I don't see why this
cannot be warnings instead.
> - {{{self._max_length = ZZ(max_length) if max_length != Infinity else
max_length}}}
> Just my two cents: I would return 'Infinity' instead of max_length.
You
> defined your `Infinity` in order to be fast, and you don't know what
> `max_length` may be. This occurs several times.
Agreed, see also `integer_or_infinity` from #17920.
> - I do not understand this comment:
> {{{If a part has no bound on its value, it will be detected at some
point}}}
> Do you mean that "at some point in __iter__" it will be detected?
Probably
> not, as this function is called only once.
Agreed that this is confusing. This comment should really be moved to and
explained the docstring.
> - The many `if A: return` from `_check_finiteness` can be replaced by a
`if A or
> B or C or ...: return`.
That's just a matter of style. The many `if A: return` might be more clear
to read...
--
Ticket URL: <http://trac.sagemath.org/ticket/17979#comment:413>
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.
