Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#8407 closed enhancement (fixed)

more practical construction of word paths

Reported by: slabbe Owned by: slabbe
Priority: major Milestone: sage-4.4.1
Component: combinatorics Keywords:
Cc: abmasse Merged in: sage-4.4.1.alpha2
Authors: Sébastien Labbé Reviewers: Alexandre Blondin Massé, Nathann Cohen
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by slabbe)

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

Attachments (1)

trac_8407_word-paths-sl.patch (4.7 KB) - added by slabbe 8 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 8 years ago by slabbe

  • Cc abmasse added
  • Status changed from new to needs_review

comment:2 Changed 8 years ago by abmasse

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.
  1. 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.
  1. 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 ?

comment:3 Changed 8 years ago by abmasse

  • Status changed from needs_review to needs_work

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

comment:4 Changed 8 years ago by slabbe

  • Description modified (diff)
  • Summary changed from word paths isometries + improve construction to more practical construction of word paths

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.

Changed 8 years ago by slabbe

comment:5 Changed 8 years ago by slabbe

  • Status changed from needs_work to needs_review

comment:6 Changed 7 years ago by ncohen

  • Status changed from needs_review to positive_review

Applies fine, does it job :-)

Thank you for your work !

Nathann

comment:7 Changed 7 years ago by was

  • Merged in set to 4.4.1.alpha2
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:8 Changed 7 years ago by mvngu

  • Authors set to Sébastien Labbé
  • Reviewers set to Alexandre Blondin Massé, Nathann Cohen

comment:9 Changed 7 years ago by mvngu

  • Merged in changed from 4.4.1.alpha2 to sage-4.4.1.alpha2
Note: See TracTickets for help on using tickets.