Sage: Ticket #14319: Automorphism group with labeled vertices
https://trac.sagemath.org/ticket/14319
<p>
Permutation groups used to support only 1...n elements, but that is not the case anymore. We can return automorphism groups that do not need to be relabeled, and that is GREAT <code>:-P</code>
</p>
<p>
Apply
</p>
<ul><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/14319/trac_14319.patch" title="Attachment 'trac_14319.patch' in Ticket #14319">trac_14319.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/14319/trac_14319.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/14319/trac_14319_fix_fan_isomorphism.patch" title="Attachment 'trac_14319_fix_fan_isomorphism.patch' in Ticket #14319">trac_14319_fix_fan_isomorphism.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/14319/trac_14319_fix_fan_isomorphism.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/14319/trac_14319-from_list_to_domain.patch" title="Attachment 'trac_14319-from_list_to_domain.patch' in Ticket #14319">trac_14319-from_list_to_domain.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/14319/trac_14319-from_list_to_domain.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/14319/trac_14319-empty.patch" title="Attachment 'trac_14319-empty.patch' in Ticket #14319">trac_14319-empty.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/14319/trac_14319-empty.patch" title="Download"></a>
</li></ul>en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/14319
Trac 1.1.6ncohenWed, 20 Mar 2013 10:53:09 GMTstatus, description changed
https://trac.sagemath.org/ticket/14319#comment:1
https://trac.sagemath.org/ticket/14319#comment:1
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/14319?action=diff&version=1">diff</a>)
</li>
</ul>
<p>
I hope the patchbot will like this one. I'm pretty sure that it will break 1000 doctests <code>:-P</code>
</p>
<p>
Nathann
</p>
TicketdimpaseWed, 20 Mar 2013 12:43:29 GMT
https://trac.sagemath.org/ticket/14319#comment:2
https://trac.sagemath.org/ticket/14319#comment:2
<p>
What are the allowed labels/doman elements of the underlying permutation groups?
Can they be, say, tuples?
</p>
TicketncohenWed, 20 Mar 2013 12:45:24 GMT
https://trac.sagemath.org/ticket/14319#comment:3
https://trac.sagemath.org/ticket/14319#comment:3
<blockquote class="citation">
<p>
What are the allowed labels/doman elements of the underlying permutation groups?
Can they be, say, tuples?
</p>
</blockquote>
<p>
Well, it lookslike it can be anything that can be hashed. A <code>PermutationGroup</code> contains its own dictionary of "Sage things" <-> "Gap Things" and does the translations, soo... <code>:-)</code>
</p>
<p>
Nathann
</p>
TicketncohenWed, 20 Mar 2013 13:01:54 GMTdependencies set
https://trac.sagemath.org/ticket/14319#comment:4
https://trac.sagemath.org/ticket/14319#comment:4
<ul>
<li><strong>dependencies</strong>
set to <em>#14291</em>
</li>
</ul>
TicketvbraunFri, 22 Mar 2013 11:39:05 GMT
https://trac.sagemath.org/ticket/14319#comment:5
https://trac.sagemath.org/ticket/14319#comment:5
<p>
Various docstrings need fixing as they explicitly refer to integers, for example <code>list()</code>:
</p>
<pre class="wiki">Returns list of the images of the integers from 1 to n under this
permutation as a list of Python ints.
</pre>
TicketncohenFri, 22 Mar 2013 11:42:09 GMT
https://trac.sagemath.org/ticket/14319#comment:6
https://trac.sagemath.org/ticket/14319#comment:6
<p>
Where does this docstring come from ?
</p>
TicketvbraunFri, 22 Mar 2013 13:44:24 GMT
https://trac.sagemath.org/ticket/14319#comment:7
https://trac.sagemath.org/ticket/14319#comment:7
<p>
Its in <code>sage/groups/perm_gps/permgroup_element.pyx</code>
</p>
TicketncohenFri, 22 Mar 2013 14:07:47 GMT
https://trac.sagemath.org/ticket/14319#comment:8
https://trac.sagemath.org/ticket/14319#comment:8
<blockquote class="citation">
<p>
Its in <code>sage/groups/perm_gps/permgroup_element.pyx</code>
</p>
</blockquote>
<p>
Just for the record, <a class="closed ticket" href="https://trac.sagemath.org/ticket/10335" title="enhancement: Add domains for permutation groups (closed: fixed)">#10335</a> is where this should have been done in the first place <code>>_<</code>
</p>
<p>
Nathann
</p>
TicketncohenFri, 22 Mar 2013 14:11:03 GMT
https://trac.sagemath.org/ticket/14319#comment:9
https://trac.sagemath.org/ticket/14319#comment:9
<p>
This being said, I have no idea what to do with this <code>list</code> method. It just has no meaning if the elements are not integers <code>:-/</code>
</p>
<p>
Nathann
</p>
TicketncohenFri, 22 Mar 2013 14:44:03 GMT
https://trac.sagemath.org/ticket/14319#comment:10
https://trac.sagemath.org/ticket/14319#comment:10
<p>
Patch updated ! I did not touch <code>list()</code>. What do you think we should do with it ? Update its docstring to say "returns the domain" ? Looks like this is all it is used for. And hopefully, because doing otherwise would assume that it is only defined on integers.
</p>
<p>
Nathann
</p>
TicketvbraunSat, 23 Mar 2013 09:23:18 GMT
https://trac.sagemath.org/ticket/14319#comment:11
https://trac.sagemath.org/ticket/14319#comment:11
<p>
I would be in favor of having a <code>domain</code> method on the permutation group elements and deprecate <code>list</code>.
</p>
TicketvbraunSat, 23 Mar 2013 09:28:02 GMTdescription changed
https://trac.sagemath.org/ticket/14319#comment:12
https://trac.sagemath.org/ticket/14319#comment:12
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/14319?action=diff&version=12">diff</a>)
</li>
</ul>
TicketncohenSat, 23 Mar 2013 10:28:44 GMTdescription changed
https://trac.sagemath.org/ticket/14319#comment:13
https://trac.sagemath.org/ticket/14319#comment:13
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/14319?action=diff&version=13">diff</a>)
</li>
</ul>
<blockquote class="citation">
<p>
I would be in favor of having a <code>domain</code> method on the permutation group elements and deprecate <code>list</code>.
</p>
</blockquote>
<p>
Done in this new patch ! Thank you for looking at <code>fan_isomorphism</code> <code>:-)</code>
</p>
<p>
Nathann
</p>
TicketncohenSat, 23 Mar 2013 10:37:54 GMTdependencies changed
https://trac.sagemath.org/ticket/14319#comment:14
https://trac.sagemath.org/ticket/14319#comment:14
<ul>
<li><strong>dependencies</strong>
changed from <em>#14291</em> to <em>#14291, #14250</em>
</li>
</ul>
<p>
Rebased on top of <a class="closed ticket" href="https://trac.sagemath.org/ticket/14250" title="enhancement: GenericGraph.is_circulant() test (closed: fixed)">#14250</a> which is in 5.9.beta1.
</p>
<p>
Nathann
</p>
TicketncohenSat, 23 Mar 2013 10:38:36 GMT
https://trac.sagemath.org/ticket/14319#comment:15
https://trac.sagemath.org/ticket/14319#comment:15
<p>
Apply trac_14319.patch, trac_14319_fix_fan_isomorphism.patch, trac_14319-from_list_to_domain.patch
</p>
TicketncohenSat, 23 Mar 2013 11:14:41 GMTattachment set
https://trac.sagemath.org/ticket/14319
https://trac.sagemath.org/ticket/14319
<ul>
<li><strong>attachment</strong>
set to <em>trac_14319-from_list_to_domain.patch</em>
</li>
</ul>
TicketvbraunMon, 01 Apr 2013 20:52:02 GMT
https://trac.sagemath.org/ticket/14319#comment:16
https://trac.sagemath.org/ticket/14319#comment:16
<p>
You forgot at least one corner case, if the automorphism group is trivial then still need to remember the domain:
</p>
<pre class="wiki">sage: Graph({'a':['a'], 'b':[]}).automorphism_group()
Permutation Group with generators [()]
sage: Graph({'a':['a'], 'b':[]}).automorphism_group().domain()
{1}
</pre>
TicketvbraunMon, 01 Apr 2013 21:04:38 GMTattachment set
https://trac.sagemath.org/ticket/14319
https://trac.sagemath.org/ticket/14319
<ul>
<li><strong>attachment</strong>
set to <em>trac_14319_fix_fan_isomorphism.patch</em>
</li>
</ul>
<p>
Updated patch
</p>
TicketncohenMon, 01 Apr 2013 23:05:57 GMTdescription changed
https://trac.sagemath.org/ticket/14319#comment:17
https://trac.sagemath.org/ticket/14319#comment:17
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/14319?action=diff&version=17">diff</a>)
</li>
</ul>
<p>
Apply trac_14319.patch, trac_14319_fix_fan_isomorphism.patch, trac_14319-from_list_to_domain.patch, trac_14319-empty.patch
</p>
TicketncohenMon, 01 Apr 2013 23:06:07 GMT
https://trac.sagemath.org/ticket/14319#comment:18
https://trac.sagemath.org/ticket/14319#comment:18
<p>
Updated.
</p>
<p>
Nathann
</p>
TicketvbraunThu, 11 Apr 2013 19:14:43 GMT
https://trac.sagemath.org/ticket/14319#comment:19
https://trac.sagemath.org/ticket/14319#comment:19
<p>
On sage-5.9.beta5 A number of doctests fail of the sort:
</p>
<pre class="wiki">File "devel/sage/sage/combinat/tableau.py", line 1613, in sage.combinat.tableau.Tableau.row_stabilizer
Failed example:
rs.one().list()
Expected:
[1, 2, 3]
Got:
doctest:1: DeprecationWarning: list is deprecated. Please use domain instead.
See http://trac.sagemath.org/14319 for details.
[1, 2, 3]
</pre>
TicketncohenThu, 11 Apr 2013 21:49:18 GMT
https://trac.sagemath.org/ticket/14319#comment:20
https://trac.sagemath.org/ticket/14319#comment:20
<p>
Updated.
</p>
<p>
Nathann
</p>
TicketvbraunWed, 17 Apr 2013 17:54:01 GMT
https://trac.sagemath.org/ticket/14319#comment:21
https://trac.sagemath.org/ticket/14319#comment:21
<p>
Still get DeprecationWarning: list is deprecated failures (sage-5.9.beta5):
</p>
<pre class="wiki">sage -t devel/sage/sage/rings/number_field/number_field.py # 1 doctest failed
sage -t devel/sage/sage/crypto/classical.py # 1 doctest failed
sage -t devel/sage/sage/combinat/tableau_tuple.py # 1 doctest failed
sage -t devel/sage/sage/combinat/words/finite_word.py # 2 doctests failed
sage -t devel/sage/sage/rings/number_field/galois_group.py # 1 doctest failed
sage -t devel/sage/sage/rings/number_field/number_field_ideal.py # 1 doctest failed
sage -t devel/sage/sage/coding/linear_code.py # 1 doctest failed
sage -t devel/sage/doc/en/bordeaux_2008/nf_galois_groups.rst # 1 doctest failed
sage -t devel/sage/sage/modular/arithgroup/arithgroup_perm.py # 1 doctest failed
sage -t devel/sage/sage/combinat/permutation.py # 2 doctests failed
sage -t devel/sage/sage/structure/sage_object.pyx # 1 doctest failed
sage -t devel/sage/sage/modular/arithgroup/farey_symbol.pyx # 1 doctest failed
sage -t devel/sage/sage/coding/binary_code.pyx # 2 doctests failed
sage -t devel/sage/sage/combinat/symmetric_group_representations.py # 1 doctest failed
sage -t devel/sage/sage/combinat/dyck_word.py # 1 doctest failed
sage -t devel/sage/sage/modular/arithgroup/congroup_generic.py # 1 doctest failed
sage -t devel/sage/sage/combinat/symmetric_group_algebra.py # 1 doctest failed
sage -t devel/sage/sage/categories/finite_permutation_groups.py # 1 doctest failed
sage -t devel/sage/sage/combinat/species/permutation_species.py # 1 doctest failed
</pre>
TicketncohenThu, 18 Apr 2013 07:54:40 GMT
https://trac.sagemath.org/ticket/14319#comment:22
https://trac.sagemath.org/ticket/14319#comment:22
<p>
What the heeeeeeeeelll ?? I tested this patch one thousand times <code>>_<</code>
</p>
<p>
Nathann
</p>
TicketncohenThu, 18 Apr 2013 08:47:43 GMT
https://trac.sagemath.org/ticket/14319#comment:23
https://trac.sagemath.org/ticket/14319#comment:23
<p>
Updated ! <code>O_O;;;;;</code>
</p>
<p>
Nathann
</p>
TicketvbraunThu, 18 Apr 2013 09:35:36 GMT
https://trac.sagemath.org/ticket/14319#comment:24
https://trac.sagemath.org/ticket/14319#comment:24
<p>
Can you just run a <code>make ptest</code>? There are still lots of places left:
</p>
<pre class="wiki">sage -t devel/sage/sage/rings/number_field/number_field.py # 1 doctest failed
sage -t devel/sage/sage/combinat/words/finite_word.py # 2 doctests failed
sage -t devel/sage/sage/combinat/tableau_tuple.py # 1 doctest failed
sage -t devel/sage/sage/coding/binary_code.pyx # 2 doctests failed
sage -t devel/sage/sage/crypto/classical.py # 1 doctest failed
sage -t devel/sage/sage/coding/linear_code.py # 1 doctest failed
sage -t devel/sage/sage/rings/number_field/galois_group.py # 1 doctest failed
sage -t devel/sage/sage/modular/arithgroup/arithgroup_perm.py # 1 doctest failed
sage -t devel/sage/sage/rings/number_field/number_field_ideal.py # 1 doctest failed
sage -t devel/sage/sage/structure/sage_object.pyx # 1 doctest failed
sage -t devel/sage/doc/en/bordeaux_2008/nf_galois_groups.rst # 1 doctest failed
sage -t devel/sage/sage/combinat/permutation.py # 2 doctests failed
sage -t devel/sage/sage/modular/arithgroup/farey_symbol.pyx # 1 doctest failed
sage -t devel/sage/sage/combinat/symmetric_group_representations.py # 1 doctest failed
sage -t devel/sage/sage/combinat/dyck_word.py # 1 doctest failed
sage -t devel/sage/sage/modular/arithgroup/congroup_generic.py # 1 doctest failed
sage -t devel/sage/sage/combinat/symmetric_group_algebra.py # 1 doctest failed
sage -t devel/sage/sage/categories/finite_permutation_groups.py # 1 doctest failed
sage -t devel/sage/sage/combinat/species/permutation_species.py # 1 doctest failed
</pre>
TicketncohenThu, 18 Apr 2013 09:39:17 GMT
https://trac.sagemath.org/ticket/14319#comment:25
https://trac.sagemath.org/ticket/14319#comment:25
<blockquote class="citation">
<p>
Can you just run a <code>make ptest</code>? There are still lots of places left:
</p>
</blockquote>
<p>
I did, and all these tests pass.
</p>
<p>
Nathann
</p>
TicketvbraunThu, 18 Apr 2013 10:05:11 GMT
https://trac.sagemath.org/ticket/14319#comment:26
https://trac.sagemath.org/ticket/14319#comment:26
<p>
Do you have any other patches applied?
</p>
<pre class="wiki">trac_14291-v2.patch
trac_14291_reviewer.patch
trac_14319.patch
trac_14319-from_list_to_domain.patch
trac_14319_fix_fan_isomorphism.patch
trac_14319-empty.patch
</pre><p>
None of these patches touches <code>sage/combinat/tableau_tuple.py</code>, for example.
</p>
TicketvbraunThu, 18 Apr 2013 10:06:04 GMT
https://trac.sagemath.org/ticket/14319#comment:27
https://trac.sagemath.org/ticket/14319#comment:27
<p>
Or you might have an old version of <code>trac_14319-from_list_to_domain.patch</code>, since this patch introduced the deprecation warning.
</p>
TicketncohenThu, 18 Apr 2013 10:11:02 GMT
https://trac.sagemath.org/ticket/14319#comment:28
https://trac.sagemath.org/ticket/14319#comment:28
<p>
<code></code>tableau_tuple<code></code> is modified by the version of <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/14319/trac_14319-empty.patch" title="Attachment 'trac_14319-empty.patch' in Ticket #14319">trac_14319-empty.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/14319/trac_14319-empty.patch" title="Download"></a> I uploaded earlier. You can check it on the html version of the patch : <a class="ext-link" href="http://trac.sagemath.org/sage_trac/attachment/ticket/14319/trac_14319-empty.patch"><span class="icon"></span>http://trac.sagemath.org/sage_trac/attachment/ticket/14319/trac_14319-empty.patch</a>
</p>
<p>
In case it may be related : the patchbot sees down. I do not know how you download the patch files, but I had to do it manually because my scripts rely on the patchbot for that... But really, the HTML version of the patch you get when you click on it, on this trac ticket, does modify <code></code>tableau_tuple<code></code>.
</p>
<p>
Nathann
</p>
TicketvbraunThu, 18 Apr 2013 10:24:29 GMT
https://trac.sagemath.org/ticket/14319#comment:29
https://trac.sagemath.org/ticket/14319#comment:29
<p>
Ok, seems like I tripped yet again over <a class="closed ticket" href="https://trac.sagemath.org/ticket/11813" title="defect: Stale caches with trac and transparent proxies (closed: wontfix)">#11813</a>
</p>
TicketvbraunThu, 18 Apr 2013 11:20:04 GMTstatus changed; reviewer set
https://trac.sagemath.org/ticket/14319#comment:30
https://trac.sagemath.org/ticket/14319#comment:30
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>reviewer</strong>
set to <em>Volker Braun</em>
</li>
</ul>
<p>
Looks good to me
</p>
TicketncohenThu, 18 Apr 2013 11:22:09 GMT
https://trac.sagemath.org/ticket/14319#comment:31
https://trac.sagemath.org/ticket/14319#comment:31
<p>
Yeahhhhhhhhhhhhhhhhhhhhhhhh!! Thank you soooooo much ! I love that patch <code>:-PPP</code>
</p>
<p>
Nathann
</p>
TicketjdemeyerFri, 19 Apr 2013 08:41:40 GMTmilestone changed
https://trac.sagemath.org/ticket/14319#comment:32
https://trac.sagemath.org/ticket/14319#comment:32
<ul>
<li><strong>milestone</strong>
changed from <em>sage-5.9</em> to <em>sage-5.10</em>
</li>
</ul>
TicketncohenFri, 19 Apr 2013 08:43:21 GMT
https://trac.sagemath.org/ticket/14319#comment:33
https://trac.sagemath.org/ticket/14319#comment:33
<p>
NOoooooooooooooooooooooooooooooooooooooo <code>:-PPPPPP</code>
</p>
<p>
Nathann
</p>
TicketjdemeyerWed, 24 Apr 2013 07:36:25 GMTstatus, dependencies changed; work_issues set
https://trac.sagemath.org/ticket/14319#comment:34
https://trac.sagemath.org/ticket/14319#comment:34
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
<li><strong>dependencies</strong>
changed from <em>#14291, #14250</em> to <em>#14291, #14250, #14477, #14435</em>
</li>
<li><strong>work_issues</strong>
set to <em>rebase</em>
</li>
</ul>
<p>
Please rebase to <a class="closed ticket" href="https://trac.sagemath.org/ticket/14477" title="defect: Graph database query results should be sorted (closed: fixed)">#14477</a> + <a class="closed ticket" href="https://trac.sagemath.org/ticket/14435" title="enhancement: A certificate for is_forest (closed: fixed)">#14435</a>.
</p>
TicketncohenWed, 24 Apr 2013 07:38:15 GMT
https://trac.sagemath.org/ticket/14319#comment:35
https://trac.sagemath.org/ticket/14319#comment:35
<p>
Comeeeee ooooooooonnnnnnnnn... This patch is hell to rebase <code>T_T</code>
</p>
<p>
Sigh... <code>T_T</code>
</p>
<p>
Will do it... <code>T_T</code>
</p>
<p>
Nathann
</p>
TicketncohenWed, 24 Apr 2013 07:49:57 GMTattachment set
https://trac.sagemath.org/ticket/14319
https://trac.sagemath.org/ticket/14319
<ul>
<li><strong>attachment</strong>
set to <em>trac_14319.patch</em>
</li>
</ul>
TicketncohenWed, 24 Apr 2013 07:51:07 GMTstatus changed
https://trac.sagemath.org/ticket/14319#comment:36
https://trac.sagemath.org/ticket/14319#comment:36
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>positive_review</em>
</li>
</ul>
<p>
Hmmmmmmm... Was not that bad after all <code>:-P</code>
</p>
<p>
Nathann
</p>
TicketncohenWed, 24 Apr 2013 07:51:17 GMT
https://trac.sagemath.org/ticket/14319#comment:37
https://trac.sagemath.org/ticket/14319#comment:37
<p>
Apply trac_14319.patch, trac_14319_fix_fan_isomorphism.patch, trac_14319-from_list_to_domain.patch, trac_14319-empty.patch
</p>
TicketjdemeyerTue, 30 Apr 2013 11:34:02 GMTwork_issues deleted
https://trac.sagemath.org/ticket/14319#comment:38
https://trac.sagemath.org/ticket/14319#comment:38
<ul>
<li><strong>work_issues</strong>
<em>rebase</em> deleted
</li>
</ul>
TicketjdemeyerSat, 04 May 2013 12:29:19 GMTstatus changed
https://trac.sagemath.org/ticket/14319#comment:39
https://trac.sagemath.org/ticket/14319#comment:39
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
On various systems:
</p>
<pre class="wiki">sage -t --long devel/sage/sage/homology/simplicial_complex.py
**********************************************************************
File "devel/sage/sage/homology/simplicial_complex.py", line 3149, in sage.homology.simplicial_complex.SimplicialComplex.automorphism_group
Failed example:
group.domain()
Expected:
{1, 2, 3, 'a'}
Got:
{'a', 1, 2, 3}
**********************************************************************
</pre>
TicketncohenSat, 04 May 2013 15:11:59 GMTstatus changed
https://trac.sagemath.org/ticket/14319#comment:40
https://trac.sagemath.org/ticket/14319#comment:40
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>positive_review</em>
</li>
</ul>
<p>
If it works with a Graph's vertices, it should work there too... <code>:-P</code>
</p>
<p>
They're now sorted in the doctest !
</p>
<p>
Nathann
</p>
<p>
P.S. : And just to make it clear, it passed all tests on my two machines... <code>:-/</code>
</p>
TicketvbraunSat, 04 May 2013 15:21:55 GMTstatus changed
https://trac.sagemath.org/ticket/14319#comment:41
https://trac.sagemath.org/ticket/14319#comment:41
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
Nonono they are already sorted in the set output. The problem is that python compares int and string by address, which depends on memory layout. The best fix for the doctest would be to have string labels for the whole domain.
</p>
TicketncohenSat, 04 May 2013 15:24:53 GMT
https://trac.sagemath.org/ticket/14319#comment:42
https://trac.sagemath.org/ticket/14319#comment:42
<p>
Oh. I thought that it would compare types. Okayokay, then !
</p>
<p>
Nathann
</p>
TicketncohenSat, 04 May 2013 15:29:09 GMT
https://trac.sagemath.org/ticket/14319#comment:43
https://trac.sagemath.org/ticket/14319#comment:43
<p>
Fixed again !
</p>
<pre class="wiki">sage: set(group.domain()) == set(Z.vertices())
True
</pre><p>
Is that ok Volker ? I don't want Jeroen to yell at me again. Nor to send to sage-devel (without naming me) all my past mistakes... <code>:-P</code>
</p>
<p>
Nathann
</p>
TicketvbraunSat, 04 May 2013 15:56:37 GMT
https://trac.sagemath.org/ticket/14319#comment:44
https://trac.sagemath.org/ticket/14319#comment:44
<p>
Thats works but is fugly.. how about changing the simplicial complex to
</p>
<pre class="wiki">sage: Z = SimplicialComplex([['1', '2'],['2', '3', 'a']])
</pre>
TicketncohenSat, 04 May 2013 16:02:14 GMTattachment set
https://trac.sagemath.org/ticket/14319
https://trac.sagemath.org/ticket/14319
<ul>
<li><strong>attachment</strong>
set to <em>trac_14319-empty.patch</em>
</li>
</ul>
TicketncohenSat, 04 May 2013 16:02:26 GMTstatus changed
https://trac.sagemath.org/ticket/14319#comment:45
https://trac.sagemath.org/ticket/14319#comment:45
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>positive_review</em>
</li>
</ul>
<p>
Feels like it's cheating. Plus it's nice to test weird inputs too. But done.
</p>
<p>
Nathann
</p>
TicketjdemeyerTue, 07 May 2013 09:06:05 GMTmerged set
https://trac.sagemath.org/ticket/14319#comment:46
https://trac.sagemath.org/ticket/14319#comment:46
<ul>
<li><strong>merged</strong>
set to <em>sage-5.10.beta2</em>
</li>
</ul>
TicketjdemeyerTue, 07 May 2013 09:07:14 GMTstatus changed; resolution set
https://trac.sagemath.org/ticket/14319#comment:47
https://trac.sagemath.org/ticket/14319#comment:47
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
</ul>
TicketnthieryMon, 04 Nov 2013 21:00:49 GMT
https://trac.sagemath.org/ticket/14319#comment:48
https://trac.sagemath.org/ticket/14319#comment:48
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14319#comment:10" title="Comment 10">ncohen</a>:
</p>
<blockquote class="citation">
<p>
Patch updated ! I did not touch <code>list()</code>. What do you think we should do with it ? Update its docstring to say "returns the domain" ? Looks like this is all it is used for. And hopefully, because doing otherwise would assume that it is only defined on integers.
</p>
</blockquote>
<p>
Damned, I missed this. The list method is important! It gives the permutation in list notation, and I am using it in my class tomorrow! Ok, the meaning is a bit dubious when the domain is any set, but I guess the following semantic would make sense:
</p>
<ul><li>domain(): returns self.parent().domain() (if at all needed)
</li><li>list(): returns [self(i) for i in self.parent().domain()]
</li><li>tuple(): returns tuple(self.list())
</li></ul><p>
If you agree with the above, I'll create a new ticket. And ignore the warning in class tomorrow ...
</p>
TicketncohenMon, 04 Nov 2013 21:22:29 GMT
https://trac.sagemath.org/ticket/14319#comment:49
https://trac.sagemath.org/ticket/14319#comment:49
<p>
Yoooooooooo !!
</p>
<blockquote class="citation">
<p>
Damned, I missed this.
</p>
</blockquote>
<p>
Looks like I added you to the Cc when I created the ticket, though. And Florent, too.
</p>
<blockquote class="citation">
<p>
The list method is important! It gives the permutation in list notation, and I am using it in my class tomorrow!
</p>
</blockquote>
<p>
Yeah well, you may need it and everything but it isn't even defined properly.
</p>
<blockquote class="citation">
<p>
Ok, the meaning is a bit dubious when the domain is any set
</p>
</blockquote>
<p>
DUBIOUS ? Come on, it has no meaning whatsoever. Other than what "domain" returns.
</p>
<blockquote class="citation">
<p>
but I guess the following semantic would make sense:
</p>
<ul><li>domain(): returns self.parent().domain() (if at all needed)
</li><li>list(): returns [self(i) for i in self.parent().domain()]
</li><li>tuple(): returns tuple(self.list())
</li></ul><p>
If you agree with the above, I'll create a new ticket. And ignore the warning in class tomorrow ...
</p>
</blockquote>
<p>
Why don't you just add some lines to the constructor of <code>Permutation</code> ? You could just write <code>p = Permutation(your_permutation_group_element)</code>. That would have a meaning.
</p>
<p>
What would a <code>.list()</code> function do in <code>PermutationGroupElement</code> otherwise ? Hell, what does <code>.tuple()</code> do there too ? It's not like <code>tuple(a.domain())</code> is so long that it needs a function of its own <code>O_o</code>
</p>
<p>
Nathann
</p>
TicketnthieryMon, 04 Nov 2013 21:49:48 GMT
https://trac.sagemath.org/ticket/14319#comment:50
https://trac.sagemath.org/ticket/14319#comment:50
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14319#comment:49" title="Comment 49">ncohen</a>:
</p>
<blockquote class="citation">
<p>
Looks like I added you to the Cc when I created the ticket, though. And Florent, too.
</p>
</blockquote>
<p>
Sure, I take the blame for not spotting this and suggesting something
different back then.
</p>
<blockquote class="citation">
<p>
DUBIOUS ? Come on, it has no meaning whatsoever. Other than what "domain" returns.
</p>
</blockquote>
<p>
Yes it does! When the domain is 1...n it gives you the permutation in
the usual list notation, which is a basic feature! And otherwise it
still shows how the elements of the domain are permuted which is a
worthwhile information; it does not seem ridiculous to me to do:
</p>
<pre class="wiki">sage: for sigma in SymmetricGroup(['a','b','c']): print sigma.list()
['a', 'b', 'c']
['a', 'c', 'b']
['b', 'a', 'c']
['b', 'c', 'a']
['c', 'a', 'b']
['c', 'b', 'a']
</pre><p>
If there is something to be contested it's that self.domain() returns
the domain with its elements permuted; I would just return the domain
of the parent (like other functions do in Sage).
</p>
TicketncohenMon, 04 Nov 2013 21:59:22 GMT
https://trac.sagemath.org/ticket/14319#comment:51
https://trac.sagemath.org/ticket/14319#comment:51
<p>
Yoooooo !
</p>
<blockquote class="citation">
<p>
Yes it does! When the domain is 1...n it gives you the permutation in
the usual list notation, which is a basic feature!
</p>
</blockquote>
<p>
I understand that this information is useful in this case, the scope of <code>PermutationGroupElement</code> is much larger than that.
</p>
<blockquote class="citation">
<p>
And otherwise it
still shows how the elements of the domain are permuted which is a
worthwhile information;
</p>
</blockquote>
<p>
Isn't that machine-dependent when domain is a set ?
</p>
<blockquote class="citation">
<p>
If there is something to be contested it's that self.domain() returns
the domain with its elements permuted; I would just return the domain
of the parent (like other functions do in Sage).
</p>
</blockquote>
<p>
No objection to that.
</p>
<p>
What would be the problem of a <code>.to_permutation</code> method (the findstat guys would love that) which would output the permutation of <code>1...n</code> corresponding to the element ? When the domain is not 1...n they could be relabeled "in any way" (i.e. the ordering of <code>.domain()</code>) to return a permutation of 1...n, and when the domain IS 1...n then it could return the list that you expect ?
</p>
<p>
Or you could also implement something in the constructor of <code>Permutation</code> which would do the same, what do you think ?
</p>
<p>
Why the hell do we have both <code>Permutation</code> and <code>PermutationGroupElement</code> by the way ?
</p>
<p>
Nathann
</p>
TicketjasonMon, 20 Jan 2014 14:33:23 GMT
https://trac.sagemath.org/ticket/14319#comment:52
https://trac.sagemath.org/ticket/14319#comment:52
<p>
I just ran into the .list() deprecation warning as well, and was a bit confused by p.domain() permuting the domain. nthiery: were you planning on opening a new ticket to have p.domain() just give back the parent's domain, and restoring the p.list()? I'm deciding whether to ignore the deprecation or to update my code.
</p>
TicketncohenMon, 20 Jan 2014 14:58:59 GMT
https://trac.sagemath.org/ticket/14319#comment:53
https://trac.sagemath.org/ticket/14319#comment:53
<p>
Ohhhhhhhh... An unanswered question from 3 months ago <code>:-P</code>
</p>
<p>
Nathann
</p>
TicketncohenMon, 20 Jan 2014 15:01:26 GMT
https://trac.sagemath.org/ticket/14319#comment:54
https://trac.sagemath.org/ticket/14319#comment:54
<p>
Jason : what would you want <code>.list()</code> to return except what <code>.domain()</code> returns ? What it returns at the moment only has a meaning for specific sets of vertices, and this class is meant to handle everything !
</p>
TicketjasonMon, 20 Jan 2014 15:09:53 GMT
https://trac.sagemath.org/ticket/14319#comment:55
https://trac.sagemath.org/ticket/14319#comment:55
<p>
My confusion is just about the naming of <code>.domain()</code>. At first, it sounded like if <code>p</code> was a permutation group element, then <code>p.domain()</code> would just give me back the original domain of the permutation group, and <code>p.list()</code> would do what it has done in the past---a list representation of the permutation (exactly what nthiery describes). Even after reading the patch, I had to double-check that domain actually did permute the domain, because it sounds like it shouldn't to me.
</p>
TicketjasonMon, 20 Jan 2014 15:11:13 GMT
https://trac.sagemath.org/ticket/14319#comment:56
https://trac.sagemath.org/ticket/14319#comment:56
<p>
Sorry---a more direct answer: it makes more sense to me if <code>.domain()</code> and <code>.list()</code> do exactly what nthiery proposes above.
</p>
TicketjasonMon, 20 Jan 2014 15:12:34 GMT
https://trac.sagemath.org/ticket/14319#comment:57
https://trac.sagemath.org/ticket/14319#comment:57
<p>
Put another way: to me, it's confusing that <code>p.parent().domain()</code> and <code>p.domain()</code> return different lists.
</p>
TicketncohenMon, 20 Jan 2014 16:03:28 GMT
https://trac.sagemath.org/ticket/14319#comment:58
https://trac.sagemath.org/ticket/14319#comment:58
<p>
Oh. I see. Well, <code>p.parent().domain()</code> probably makes more sense indeed. Then I would vote for removing <code>.domain()</code> too. My point is that you cannot give a meaning to <code>.list()</code> unless for "the subset of groups that are of personal interest to you" (i.e. those that act on integers) and for this reason I do not think this function should exist. I mean, of course you can define "list" but you cannot define what is of interest to you, i.e. that the output of <code>.list()</code> defines the permutation itself. This only works when all elements are integers, and there already is a class for that, i.e. permutations.
</p>
<p>
How are <code>p.parent().domain()</code> and <code>p.domain()</code> different ? If it is only the order, there is nothing wrong in that. Those things are morally "sets", and only returned as tuples/lists for efficiency reasons I guess.
</p>
<p>
Nathann
</p>
TicketjasonMon, 20 Jan 2014 16:11:00 GMT
https://trac.sagemath.org/ticket/14319#comment:59
https://trac.sagemath.org/ticket/14319#comment:59
<p>
I'm confused. What is the problem with renaming the current <code>p.domain()</code> to <code>p.list()</code>, and saying <code>p.list()</code> is going to return the list representation of the permutation (in terms of the domain)? That's how it always was---but before, the domain was always [1...n]. In other words, if you aren't using the domain argument of the permutation group, p.list() does what it always has done. If you are using the domain argument, it makes sense that <code>p.list()</code> will speak in terms of the domain.
</p>
TicketncohenMon, 20 Jan 2014 16:20:59 GMT
https://trac.sagemath.org/ticket/14319#comment:60
https://trac.sagemath.org/ticket/14319#comment:60
<blockquote class="citation">
<p>
I'm confused. What is the problem with renaming the current <code>p.domain()</code> to <code>p.list()</code>, and saying <code>p.list()</code> is going to return the list representation of the permutation (in terms of the domain)? That's how it always was---but before, the domain was always [1...n]. In other words, if you aren't using the domain argument of the permutation group, p.list() does what it always has done. If you are using the domain argument, it makes sense that <code>p.list()</code> will speak in terms of the domain.
</p>
</blockquote>
<p>
I am only worried of what I saw in the code of Permutations : that there are assumptions made by some developpers which are not known by others, and that code will end up being broken because the assumptions are never checked. <code>.list()</code> does not *mean* that "the output represents the permutations when the elements are integers". Having a <code>.to_permutation()</code> function which would return a <code>Permutation</code> object (i.e. that only handle integers) and a dictionary associating the elements of the <code>PermutationGroupElement</code> to integers is okay.
</p>
<p>
I just want to be sure that the code stays correct. Right now the list that is returned by <code>.list()</code> is a private attribute, and with a name like that there is absolutely no reason to believe that a code which creates this private attribute should ensure the property that you expect.
</p>
<p>
Exactly like the labeling of a poset's vertices with integers is a linear extension : that is assumed nearly everywhere in poset code, and you will sweat for a while before finding it out. If this variable was named "linear_extension" there would be no problem, though.
</p>
<p>
And changing the code of <code>list</code> to ensure that the list it returns satisfies the property that you expect would ensure that the code is good, but <code>.list()</code> really isn't right name for such a function. If you need "the integer permutation represented by a <code>PermutationGroupElement</code>, it looks like what you want is a way to create a Permutation object from a <code>PermutationGroupElement</code>.
</p>
<p>
Well. What do you think ?
</p>
<p>
Nathann
</p>
Ticket