Opened 5 years ago

Closed 5 years ago

#17104 closed enhancement (fixed)

IncidenceStructure.relabel() (no arguments)

Reported by: ncohen Owned by:
Priority: major Milestone: sage-6.4
Component: combinatorial designs Keywords:
Cc: vdelecroix, dimpase Merged in:
Authors: Nathann Cohen Reviewers: Vincent Delecroix
Report Upstream: N/A Work issues:
Branch: 8615954 (Commits) Commit: 8615954a35fdacc6ba0dfe1573ac0e942aeb496e
Dependencies: Stopgaps:

Description

One can relabel a graph to 0,...,n-1 by just calling .relabel(), and I found myself typing IS.relabel({v:i for i,v in enumerate(IS.ground_set())}) very often these last days.

With this patch one can relabel the point automatically to 0,...,n-1.

Nathann

Change History (10)

comment:1 Changed 5 years ago by ncohen

  • Branch set to u/ncohen/17104
  • Status changed from new to needs_review

comment:2 Changed 5 years ago by git

  • Commit set to 795cc45a9c7b320d326b48fb22e740cb40b93e75

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

795cc45trac #17104: IncidenceStructure.relabel() (no arguments)

comment:3 Changed 5 years ago by ncohen

  • Cc dimpase added

comment:4 Changed 5 years ago by vdelecroix

  • Branch changed from u/ncohen/17104 to u/vdelecroix/17104
  • Commit changed from 795cc45a9c7b320d326b48fb22e740cb40b93e75 to 3ade030d09adfd591c1a8f8bc18ddbe18fe5a43c

Hello,

Compared with the possibilities from graph:

  • what about a inplace argument?
  • what about accepting lists as input. The following works with graphs
    sage: G = Graphs(... whatever on [0,1,2,3] ...)
    sage: G.relabel([2,0,1,3])
    

In the branch I updated (rebase over sage-4.6) I added a commit which do these changes. I also added a nice test which illustrate the usefulness of these additions. What do you think ?

Vincent


New commits:

c081fd0trac #17104: IncidenceStructure.relabel() (no arguments)
3ade030trac #17104: allow list/tuple + add inplace arg

comment:5 Changed 5 years ago by vdelecroix

PS: If you have more work based upon this branch I will instead base my commit over yours after a merge of 6-4.beta6 (switching between beta4/beta6 is a mess).

comment:6 Changed 5 years ago by ncohen

Hellooooo !

Well, I don't think I have anything based atop of that. Though you seem to need a 'copy' function that was added in #17047. Though I suppose that copy will call it.

However, it would be cool if you could indent the description of the different types for inplace so that it is a member of the item that begins with ``inplace`` -- If ``True`` then ....

Also, there is no .points() function, what we have is ground_set.

Short of that it's all fine, thanks !

Nathann

comment:7 Changed 5 years ago by git

  • Commit changed from 3ade030d09adfd591c1a8f8bc18ddbe18fe5a43c to 8615954a35fdacc6ba0dfe1573ac0e942aeb496e

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

8615954trac #17104: reviewer comments

comment:8 Changed 5 years ago by ncohen

Looks good ! You can set the ticket to positive_review is you see nothing wrong here anymore.

Nathann

comment:9 Changed 5 years ago by vdelecroix

  • Reviewers set to Vincent Delecroix
  • Status changed from needs_review to positive_review

Doc compiles and it looks fine!

comment:10 Changed 5 years ago by vbraun

  • Branch changed from u/vdelecroix/17104 to 8615954a35fdacc6ba0dfe1573ac0e942aeb496e
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.