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.

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

Enhance method from_shape_and_word in tableau to allow English reading order #11314

Closed
hughrthomas opened this issue May 8, 2011 · 6 comments

Comments

@hughrthomas
Copy link

Enhance method from_shape_and_word in tableau to allow English reading order.

CC: @sagetrac-sage-combinat

Component: combinatorics

Keywords: days30, tableaux

Author: Hugh Thomas

Reviewer: Anne Schilling

Merged: sage-4.7.1.alpha3

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

@anneschilling
Copy link

comment:1

Attachment: trac_11314-tableau-from-shape-and-word-ht.patch.gz

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

@anneschilling
Copy link

Reviewer: Anne Schilling

@nthiery
Copy link
Contributor

nthiery commented May 9, 2011

comment:3

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

@anneschilling
Copy link

@anneschilling
Copy link

comment:4

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

@jdemeyer
Copy link

jdemeyer commented Jun 8, 2011

Merged: sage-4.7.1.alpha3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants