Opened 6 years ago

Closed 6 years ago

#14884 closed defect (fixed)

Multiplication by identity in tableau.py and tableau_tuple.py

Reported by: darij Owned by: tbd
Priority: minor Milestone: sage-5.12
Component: combinatorics Keywords: combinat, tableau, tableau tuple
Cc: tscrim, sage-combinat Merged in: sage-5.12.beta5
Authors: Darij Grinberg Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #14101 Stopgaps:

Description

This is similar to #14883 in that it removes a multiplication of a permutation by the identity permutation. This time the multiplication was not useless (it was meant to coerce the permutation into a symmetric group of large enough size), but that is easier done by just concatenating lists (given that the result is only needed as a list). There is a minor speedup (577µs to 496µs on the one of the doctested examples), but the purpose of this patch is really to replace a hack by a more logical manipulation.

Travis, feel free to fold this into your patch if you wish.

Attachments (1)

trac_14884-tableau_times_id-dg.patch (1.6 KB) - added by darij 6 years ago.
corrected version

Download all attachments as: .zip

Change History (8)

comment:1 Changed 6 years ago by darij

  • Status changed from new to needs_review

comment:2 Changed 6 years ago by darij

  • Cc sage-combinat added
  • Component changed from PLEASE CHANGE to combinatorics

comment:3 Changed 6 years ago by darij

  • Dependencies changed from 14101 to #14101

comment:4 Changed 6 years ago by tscrim

  • Reviewers set to Travis Scrimshaw

Hey Darij,

I believe taking the copy slice w[:] of w is unnecessary.

sage: L = [1, 2, 3]      
sage: k = L + [3, 5, 6]
sage: L
[1, 2, 3]

Could you remove this and repost?

Thanks,
Travis

Changed 6 years ago by darij

corrected version

comment:5 Changed 6 years ago by darij

Good point; fixed. Thanks for checking this.

comment:6 Changed 6 years ago by tscrim

  • Status changed from needs_review to positive_review

Looks good to me. Thanks Darij.

comment:7 Changed 6 years ago by jdemeyer

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