sagemath / sage

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

more practical construction of word paths #8407

Closed seblabbe closed 14 years ago

seblabbe commented 14 years ago

Improve the construction of word path parent : creation from 2*n letters and n vectors now works (it takes the opposite of vectors).

CC: @sagetrac-abmasse

Component: combinatorics

Author: Sébastien Labbé

Reviewer: Alexandre Blondin Massé, Nathann Cohen

Merged: sage-4.4.1.alpha2

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

a0e7ac7d-d8f3-4e97-aca8-5ab47a193e00 commented 14 years ago
comment:2

These functions are really interesting ! I can't wait to use them. However, here are some comments:

  1. I think this patch is a good occasion to add functions such as rotate() and reflects() (with pertinent parameters) that compute ONE rotated or reflected version of the path instead of all EIGHT at the same time. This wouldn't be too long to do and then your function isometries() could call them.

  2. I don't understand why you use the parameter reversal. If I understand it well, it is the word reversal operator, which can be geometrically interpreted as performing a rotation of angle pi (of the path) together with an orientation reversal of the path. It seems more natural to me that the parameter reversal correspond simply to the orientation reversal rather than to the word reversal.

  3. I noticed that you do not use the word "self" while documenting, but you use "path" or other similar words. I'm not sure which one is a good practice, but I think it is better to use the first one (I'm really not sure about it, so maybe you can correct me).

What do you think ?

a0e7ac7d-d8f3-4e97-aca8-5ab47a193e00 commented 14 years ago
comment:3

Just noticed I should have set this ticket to "needs work". Done.

seblabbe commented 14 years ago

Description changed:

--- 
+++ 
@@ -1,3 +1 @@
-1. Add a function that returns the isometries of word paths on the square grid.
-
-2. Improve the construction of word path parent : creation from 2*n letters and n vectors now works (it takes the opposite of vectors).
+Improve the construction of word path parent : creation from 2*n letters and n vectors now works (it takes the opposite of vectors).
seblabbe commented 14 years ago
comment:4

I removed one of the objectives of the ticket related to isometries. Indeed, I need this function for another problem so I think its use will be more understood in context. So that is why I removed this part from this ticket. I will create a new ticket for it soon.

seblabbe commented 14 years ago

Attachment: trac_8407_word-paths-sl.patch.gz

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

Applies fine, does it job :-)

Thank you for your work !

Nathann

williamstein commented 14 years ago

Merged: 4.4.1.alpha2

7c09a680-e216-4024-bb8e-9bfd4aa7f313 commented 14 years ago

Reviewer: Alexandre Blondin Massé, Nathann Cohen

7c09a680-e216-4024-bb8e-9bfd4aa7f313 commented 14 years ago

Author: Sébastien Labbé

7c09a680-e216-4024-bb8e-9bfd4aa7f313 commented 14 years ago

Changed merged from 4.4.1.alpha2 to sage-4.4.1.alpha2