#13742: No Permutation should be created that its method cannot handle
---------------------------------+------------------------------------------
Reporter: ncohen | Owner: sage-combinat
Type: defect | Status: needs_review
Priority: major | Milestone: sage-5.6
Component: combinatorics | Resolution:
Keywords: | Work issues:
Report Upstream: N/A | Reviewers:
Authors: Nathann Cohen | Merged in:
Dependencies: | Stopgaps:
---------------------------------+------------------------------------------
Description changed by ncohen:
Old description:
> The code from sage.combinat.permutation is very bad. Bad input is never
> checked, and when ones decides that Permutations are only defined on
> integers and do not contain 0, bad input happens. The code implicitly
> assumes that the elements are 1...n despite that, and the
> robinson_schensted function (no idea of what it does) definitely uses it
> on something different (i.e. lists, on which no total order is even
> defined !).
>
> This patch was meant to convince me that Sage could be trusted, and after
> wasting several hours on this I just *DO NOT* trust what is contained in
> the Combinat branch. It has been explained to me that "the code depends
> on CombinatorialObject? which is deprecated -- not as we usually
> deprecate code, but by comments in its documentation -- and that it could
> not be deprecated formally (with warnings, exceptions, ...) because a lot
> of people use it." And it seems to have been the case for the last 5
> years.
>
> Nothing is done to check that. And I will not. This patch is an attempt
> to convince me that I can use the Permutation class. That is all. It
> checks stuff, adds warnings everywhere, and documentation.
>
> Honestly, it makes me sick to think such a thing is in Sage. That's bad
> work, period.
>
> Two comments :
>
> For those who want to use the code as before, even when to handle things
> it is not meant to handle, use check_input=True as a flag when creating a
> partition Object. And if you NEED to use this, you may as well write the
> code you need properly. Don't share code that you cannot trust.
> from_cycles considers that the empty permutation is [], while
> Permutation([]) is [1]. It does not make any sense. I'm sick of it. This
> is bad work too. I don't care what people prefer, but please take a
> decision and stick with it.
> Nathann
New description:
The code from sage.combinat.permutation is very bad. Bad input is never
checked, and when ones decides that Permutations are only defined on
integers and do not contain 0, bad input happens. The code implicitly
assumes that the elements are 1...n despite that, and the
robinson_schensted function (no idea of what it does) definitely uses it
on something different (i.e. lists, on which no total order is even
defined !).
This patch was meant to convince me that Sage could be trusted, and after
wasting several hours on this I just *DO NOT* trust what is contained in
the Combinat branch. It has been explained to me that "the code depends on
CombinatorialObject? which is deprecated -- not as we usually deprecate
code, but by comments in its documentation -- and that it could not be
deprecated formally (with warnings, exceptions, ...) because a lot of
people use it." And it seems to have been the case for the last 5 years.
Nothing is done to check that. And I will not. This patch is an attempt to
convince me that I can use the Permutation class. That is all. It checks
stuff, adds warnings everywhere, and documentation.
Honestly, it makes me sick to think such a thing is in Sage. That's bad
work, period.
Two comments :
* For those who want to use the code as before, even when to handle things
it is not meant to handle, use check_input=True as a flag when creating a
partition Object. And if you NEED to use this, you may as well write the
code you need properly. Don't share code that you cannot trust.
* from_cycles considers that the empty permutation is [], while
Permutation([]) is [1]. It does not make any sense. I'm sick of it. This
is bad work too. I don't care what people prefer, but please take a
decision and stick with it.
Nathann
--
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/13742#comment:4>
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 post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
