Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add to_digraph method to Permutation #30214

Closed
slel opened this issue Jul 24, 2020 · 14 comments
Closed

Add to_digraph method to Permutation #30214

slel opened this issue Jul 24, 2020 · 14 comments

Comments

@slel
Copy link
Member

slel commented Jul 24, 2020

Before this ticket, the show method for a permutation

  • computes a graph corresponding to that permutation
  • displays it
  • but does not return it

This ticket adds a to_digraph method to return the graph
and changes the show method to use it.

This allows the following:

sage: p = Permutation([3, 1, 2])
sage: g = p.to_digraph()
sage: g.edges(labels=False)
[(1, 3), (2, 1), (3, 2)]

Inspired by a question by Ask Sage user magviana:

CC: @slel

Component: combinatorics

Keywords: permutation

Author: David Coudert

Branch/Commit: 12cadec

Reviewer: Sébastien Labbé

Issue created by migration from https://trac.sagemath.org/ticket/30214

@slel slel added this to the sage-9.2 milestone Jul 24, 2020
@dcoudert
Copy link
Contributor

Author: David Coudert

@dcoudert
Copy link
Contributor

comment:1

This should do what you want.


New commits:

498f093trac #30214: add method to_digraph to Permutation

@dcoudert
Copy link
Contributor

Commit: 498f093

@dcoudert
Copy link
Contributor

Branch: public/combinat/30214_to_digraph

@dcoudert dcoudert changed the title Add graph method to permutations Add to_digraph method to Permutation Jul 25, 2020
@seblabbe
Copy link
Contributor

comment:3

Personally, I would do

-E = [(i + 1, self[i]) for i in range(len(self))]
+E = list(enumerate(self, start=1)))

Do you agree?

@seblabbe
Copy link
Contributor

Reviewer: Sébastien Labbé

@slel
Copy link
Member Author

slel commented Aug 14, 2020

comment:5

Replying to @seblabbe:

Personally, I would do

-E = [(i + 1, self[i]) for i in range(len(self))]
+E = list(enumerate(self, start=1)))

Do you agree?

+1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 14, 2020

Changed commit from 498f093 to 12cadec

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 14, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

08c3e05trac #30214: merged with 9.2.beta8
12cadectrac #30214: use enumerate and avoid creation of list

@dcoudert
Copy link
Contributor

comment:7

Indeed a better solution. I'm also avoiding the creation of the list.

@seblabbe
Copy link
Contributor

comment:8

good, I was not sure if it takes an iterable as well. even better.

@dcoudert
Copy link
Contributor

comment:9

When the input format is specified, you can give an iterator. Otherwise...

@slel

This comment has been minimized.

@vbraun
Copy link
Member

vbraun commented Aug 20, 2020

Changed branch from public/combinat/30214_to_digraph to 12cadec

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants