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.

Sign up for GitHub

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

Jump to bottom

Caching canonical_label in IncidenceStructure.is_isomorphic #18200

Closed
nathanncohen mannequin opened this issue Apr 15, 2015 · 12 comments
Closed

Caching canonical_label in IncidenceStructure.is_isomorphic #18200

nathanncohen mannequin opened this issue Apr 15, 2015 · 12 comments
Labels
c: combinatorial designs p: major / 3 t: enhancement
Milestone
sage-6.7

Comments

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Apr 15, 2015

As we compute the information anyway, let us cache it.

Nathann

CC: @videlec

Component: combinatorial designs

Author: Nathann Cohen

Branch/Commit: 75326b0

Reviewer: Vincent Delecroix

Issue created by migration from https://trac.sagemath.org/ticket/18200
@nathanncohen nathanncohen mannequin added this to the sage-6.7 milestone Apr 15, 2015
@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Apr 15, 2015

Branch: public/18200

@nathanncohen nathanncohen mannequin added the s: needs review label Apr 15, 2015
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 15, 2015

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

668a71etrac #18200: Caching canonical_label in IncidenceStructure.is_isomorphic

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 15, 2015

Commit: 668a71e

@videlec
Copy link
Contributor

videlec commented Apr 15, 2015

comment:3

Would you remove

.. NOTE::

    If you need to test isomorphisms between one incidence
    structure and many others, you should consider using
    :meth:`canonical_label` instead of this function.

in the documentation of is_isomorphic?

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Apr 15, 2015

comment:4

O_o

Yes of course!

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 15, 2015

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

75326b0trac #18200: Review

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 15, 2015

Changed commit from 668a71e to 75326b0

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Apr 15, 2015

comment:6

Wow. Delay between your comment and my commit: 70 seconds :-P

@videlec
Copy link
Contributor

videlec commented Apr 15, 2015

comment:7

Much more natural to use now!

@videlec
Copy link
Contributor

videlec commented Apr 15, 2015

Reviewer: Vincent Delecroix

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Apr 15, 2015

comment:8

My only regret is that I cannot be too harsh on the idiot that copied the structure BEFORE computing the canonical label, yet thought about adding this warning.

Idiots.

Nathann

@vbraun
Copy link
Member

vbraun commented Apr 15, 2015

Changed branch from public/18200 to 75326b0

@vbraun vbraun closed this as completed in 0ce9976 Apr 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c: combinatorial designs p: major / 3 t: enhancement
Projects
None yet
Milestone
sage-6.7
Development

No branches or pull requests
2 participants
@vbraun @videlec