search

sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.31k stars 449 forks source link

DeBruijn digraph fixes #10547

Closed eviatarbach closed 13 years ago

eviatarbach commented 13 years ago

This patch fixes some problems with the DeBruijn digraph. Specifically, it changes alphabets to start with 0 (as is conventional), switches the variables k and n (again, convention) and removes the "words: " from the labels.

Component: graph theory

Author: Eviatar Bach

Reviewer: Nathann Cohen

Merged: sage-4.6.2.alpha1

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

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 13 years ago
comment:2

Hello !!

Could you use w.string_rep() instead or repr(w)[6:] ? This would prevent us from some trouble if the __repr__ method from words is changed later on :-)

Nathann

eviatarbach commented 13 years ago
comment:3

Hello,

Thank you, I did not know about this method! The new patch is uploaded.

Cheers!

eviatarbach commented 13 years ago

Attachment: 15057.patch.gz

New patch

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 13 years ago
comment:4

Well, it was the only fault I could find in this patch ! Thank you for fixing this ! :-)

Nathann

eviatarbach commented 13 years ago
comment:5

And thank you for reviewing!

By the way, since I'm not familiar with the review system, what are the requirements for inclusion? Does the patch get implemented once it has a positive review?

jdemeyer commented 13 years ago

Reviewer: Nathann Cohen

jdemeyer commented 13 years ago

Merged: sage-4.6.2.alpha1