Opened 7 years ago
Closed 7 years ago
#12872 closed enhancement (fixed)
A show method for permutations
Reported by: | ncohen | Owned by: | sage-combinat |
---|---|---|---|
Priority: | major | Milestone: | sage-5.1 |
Component: | combinatorics | Keywords: | |
Cc: | wdj, fhivert, sage-combinat | Merged in: | sage-5.1.beta0 |
Authors: | Nathann Cohen | Reviewers: | David Coudert |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
Following the discussion that took place there [1], this patch adds a .show() method to the Permutation class, to obtain drawings like the own we can see there [2].
Nathann
[1] https://groups.google.com/d/topic/sage-combinat-devel/vdfE7iaJTxs/discussion
[2] http://upload.wikimedia.org/wikipedia/en/7/78/Permutation_graph.svg
Attachments (8)
Change History (36)
comment:1 Changed 7 years ago by
- Description modified (diff)
comment:2 Changed 7 years ago by
- Cc wdj fhivert added
- Status changed from new to needs_review
comment:3 Changed 7 years ago by
- Description modified (diff)
comment:4 follow-up: ↓ 5 Changed 7 years ago by
Testing now, but some comments:
(1) I don't understand the line 1166 (inversion_list = []). Am I missing something?
(2) In the examples, there is only one:-) What do you think of the following idea: In the examples, compare your graph with what you get using show BipartiteGraph?, as below? Note the difference in the orientation of the graph. (I prefer the orientation in the bipartite graph plot personally.) Just food for thought. I'm attaching two plots, one is your plot, re-oriented to match the graph plot, and the other is the graph plot.
sage: p = Permutations(10).random_element(); p [5, 10, 4, 8, 3, 1, 9, 6, 2, 7] sage: E = [(i, 10+p(i)) for i in range(1,11)] sage: G = BipartiteGraph(E); G.show() sage: p.show() sage: G = BipartiteGraph(E); G.show() sage: p.show()
comment:5 in reply to: ↑ 4 Changed 7 years ago by
(1) I don't understand the line 1166 (inversion_list = []). Am I missing something?
*Sigh*
No, you are right :-)
(2) In the examples, there is only one:-) What do you think of the following idea:
Right ! What do you think of this patch ? :-)
Nathann
comment:6 follow-up: ↓ 7 Changed 7 years ago by
Here it is !
comment:7 in reply to: ↑ 6 Changed 7 years ago by
Replying to ncohen:
Here it is !
Thanks very much! However, the labels (0-9) do not match the permutation set (1-10). Shouldn't they match?
comment:8 Changed 7 years ago by
Well, for me the problems is that permutations do not match the natural choice, which is 0-9 :-P
But you are right of course, and it is now fixed.
Nathann
comment:9 Changed 7 years ago by
Hi Nathann,
Your drawing are very cool ! I'm not completely sure this is what I expected by typing p.show()
. Maybe, it's worth asking on sage-combinat-devel for a better name.
Florent
comment:10 Changed 7 years ago by
- Cc sage-combinat added
comment:11 Changed 7 years ago by
Well, I am not sure either and perhaps you have in mind another representation that would fit well in Permutation.show. What I know though, is that the drawing we have now would *NEVER* be found by an interested user unless it can be reached by Permutation.show (I do not even know if this drawing has a name, Jason says that the name he uses is non-standard, ...), and so the best for me is to have some arguments in Permutation.show() that would yield different kinds of drawings.
Do we get this patch inside of Sage while we wait for another type of drawing to be added ? I'm writing to sage-combinat-devel in the meantime !
Nathann
Changed 7 years ago by
Changed 7 years ago by
Changed 7 years ago by
Changed 7 years ago by
Changed 7 years ago by
comment:12 Changed 7 years ago by
This last patch passed all tests on 5.0.b10. I like the drawings now:-)
comment:13 follow-up: ↓ 14 Changed 7 years ago by
Hello,
I'm able to install the patch on 5.0.beta14, but shouldn't you had some tests to the function?
Also:
- paralell -> parallel
- why are you using "chord-diagram" instead of "circular"?
- You could use capitalize() in tests to ease usage.
comment:14 in reply to: ↑ 13 Changed 7 years ago by
Hellooooooooo !!!
but shouldn't you had some tests to the function?
Well, there are 4 ! Of course the image are not "checked", but the code is run anyway and nothing crashes.. That's more or less all we can do with the docstring system :-/
- paralell -> parallel
Arggggggggggggggggggggg !!!
- why are you using "chord-diagram" instead of "circular"?
Oh. Well, because it is the word that the guys from Sage-combinat picked the most naturally. I like permutations but that's their stuff much more than mine, so that they hold for True is true for me :-D
- You could use capitalize() in tests to ease usage.
Hmmmm. Well, I think that it had some meaning to do this for our LP solvers because we never know which letters should be upper case and which one are not (Gurobi or GUROBI ? Cplex or CPLEX), but let's not add a layer of administration to *all* the functions that take a parameter like that. I added a "else" statement though, so that there is an exception raised if the given string does not match any of the good ones. These arguments are always lower-case anyway.
Patch updated !
Nathann
comment:15 Changed 7 years ago by
OK (install, tests, docbuild, etc.), so it remains to wait for answers from sage-combinat.
comment:16 Changed 7 years ago by
? To what do you expect answers ?
comment:17 Changed 7 years ago by
It's written above: "Do we get this patch inside of Sage while we wait for another type of drawing to be added ? I'm writing to sage-combinat-devel in the meantime ! "
So what's the feedback?
comment:18 Changed 7 years ago by
Oh, sorry. Of course when you re-upload a patch all the previous times when you uploaded it disappear. Actually, in this patch there was at first only the "braid" drawing, and people on sage-combinat said that the most natural was to plot th disjoint union of circuits, and also the chord-diagram. So I updated my patch like that and the default drawing is now the circuit thing, with the braid only as an option. The braid was the only one implemented at first, and the only one I was personally interested in having :-)
Nathann
comment:19 Changed 7 years ago by
- Reviewers set to David Coudert
I was about to give positive review when I saw line 1169 "orizontally". Please add an 'h' ;-)
comment:20 Changed 7 years ago by
Done !
comment:21 Changed 7 years ago by
No!
comment:22 Changed 7 years ago by
Right... qrefresh :-P
Changed 7 years ago by
comment:23 Changed 7 years ago by
- Status changed from needs_review to positive_review
Nice. For me the patch is OK, so I give positive review. D.
comment:24 Changed 7 years ago by
Thaaaaaaaaaaanks !!!
Nathann
comment:25 Changed 7 years ago by
comment:26 follow-up: ↓ 27 Changed 7 years ago by
Hi Natann,
thanks for working on that! Here are some further comments:
- for the braid representation: I would think of options for orientation being X-to-Y for X,Y being top,bottom or bottom,top or left-right or right-left. Also, I think of a braid in a different way: order the inversions in some order (say lex) and then swap two strings at a time. This produces pictures like on the wiki page for the braid group, http://en.wikipedia.org/wiki/Braid_group.
- A second way of drawing the chord diagram is by having two vertices (i,"in") and (i,"out") for each letter. The pic then becomes a matching of 2n points placed on a circle.
- do you think of implementing the Rothe (or inversion) diagram Florent mentioned?
- The one-line notation is also nice when just drawing arcs between points.
Maybe at least points 3 and 4 might only be worth implementing when doing it in latex/tikz - but this might also apply to 1.
Feel free to say that you don't want to do it, I might then start looking at it one day...
Best, Christian
comment:27 in reply to: ↑ 26 Changed 7 years ago by
Hellooooooo Christian !!
- for the braid representation: I would think of options for orientation being X-to-Y for X,Y being top,bottom or bottom,top or left-right or right-left. Also, I think of a braid in a different way: order the inversions in some order (say lex) and then swap two strings at a time. This produces pictures like on the wiki page for the braid group, http://en.wikipedia.org/wiki/Braid_group.
You mean the kind of drawings produced by my patch on pseudolines (#12090) ? :-P
I will not code that twice, but that feature of pseudolines can be exposed in the .show method of course !
- A second way of drawing the chord diagram is by having two vertices (i,"in") and (i,"out") for each letter. The pic then becomes a matching of 2n points placed on a circle.
- do you think of implementing the Rothe (or inversion) diagram Florent mentioned?
- The one-line notation is also nice when just drawing arcs between points.
Actually I only needed to plot permutations as done by the initial patch, that is the algorithm="braid"
version of the current patch. I implemented others because it did not seem to be the universal way to draw them but besides that I will stay as clear from graphical codes as I can
:-P
Have fuuuuuuuuuuuuuuuuuuuuuuuuuuuun !!!
Nathann
comment:28 Changed 7 years ago by
- Merged in set to sage-5.1.beta0
- Resolution set to fixed
- Status changed from positive_review to closed
Now needs a review ! And comments too, if you do not agree with the patch
:-)
Nathann