Opened 7 years ago

Closed 6 years ago

#11314 closed enhancement (fixed)

Enhance method from_shape_and_word in tableau to allow English reading order

Reported by: hthomas Owned by: sage-combinat
Priority: major Milestone: sage-4.7.1
Component: combinatorics Keywords: days30, tableaux
Cc: sage-combinat Merged in: sage-4.7.1.alpha3
Authors: Hugh Thomas Reviewers: Anne Schilling
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

Enhance method from_shape_and_word in tableau to allow English reading order.

Attachments (2)

trac_11314-tableau-from-shape-and-word-ht.patch (2.0 KB) - added by hthomas 7 years ago.
trac_11314-tableau-from-shape-and-word-final.patch (2.2 KB) - added by aschilling 7 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 7 years ago by aschilling

  • Reviewers set to Anne Schilling
  • Status changed from new to needs_review

This patch adds the feature of transforming words to tableaux from the English reading order. I checked that all tests pass in the combinat folder. If buildbot agrees that all tests pass, this is a positive review.

Anne

comment:2 Changed 7 years ago by aschilling

  • Status changed from needs_review to positive_review

comment:3 Changed 7 years ago by nthiery

  • Status changed from positive_review to needs_work

Hi Hugh, Anne!

Thanks for the code and the review. +1 on the new option if you have a use case for it.

However, please change the interface to something like:

    sage: from_shape_and_word(shape, word, order="french")    # The default
    sage: from_shape_and_word(shape, word, order="english")

This is more expressive than:

    sage: from_shape_and_word(shape, word, False)

and will allow for later adding other reading orders (like columnwise). Besides, that's consistent with what we had (or will eventually have) for trees (order = "infix", "prefix").

As for the implementation, one can avoid duplicating the core code using:

if french:

shape = reversed(shape)

for l in shape:

res.append( list(w[j:j+l]) ) j += l

if french:

res.reverse()

Notes:

  • I got rid of the shape[i] at the occasion
  • I did not test it!

Last point: did you check how this commuted with Jason's patch?

Cheers,

Nicolas

comment:4 Changed 7 years ago by aschilling

  • Status changed from needs_work to positive_review

Thanks, Nicolas for the suggestions! I uploaded the new version and set it back to positive review.

Yes, on the sage-combinat queue I updated Jason's tableaux category patch and it commutes with Hugh's patch.

Best,

Anne

Apply: trac_11314-tableau-from-shape-and-word-final.patch

comment:5 Changed 6 years ago by jdemeyer

  • Merged in set to sage-4.7.1.alpha3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.