Opened 7 years ago

Closed 7 years ago

#14101 closed enhancement (fixed)

Remove CombinatorialClass from skew*

Reported by: tscrim Owned by: sage-combinat
Priority: major Milestone: sage-5.12
Component: combinatorics Keywords: days45, days49
Cc: sage-combinat, zabrocki, nthiery, alubovsky Merged in: sage-5.12.beta4
Authors: Travis Scrimshaw, Arthur Lubovsky Reviewers: Travis Scrimshaw, Arthur Lubovsky
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #7983, #14772, #13589, #14907, #10630 Stopgaps:

Description (last modified by tscrim)

Removing CombinatorialClass from combinat/skew_partition.py and combinat/skew_tableau.py.

Part of #12913.

Apply: trac_14101-remove_cc_skew-ts.patch​

Attachments (2)

trac_14772-review-dg.patch​ (43.6 KB) - added by darij 7 years ago.
still unfinished review patch
trac_14101-remove_cc_skew-ts.patch​ (220.9 KB) - added by tscrim 7 years ago.

Download all attachments as: .zip

Change History (31)

comment:1 Changed 7 years ago by tscrim

  • Component changed from PLEASE CHANGE to combinatorics
  • Keywords sage-combinat removed

comment:2 Changed 7 years ago by tscrim

  • Authors changed from Travis Scrimshaw to Travis Scrimshaw, Arthur Lubovsky
  • Cc nthiery alubovsky added
  • Dependencies set to #14772
  • Keywords days49 added
  • Reviewers set to Travis Scrimshaw, Arthur Lubovsky
  • Status changed from new to needs_review

The dependency on #14772 is some fuzz due to editing of cython files in symmetrica.

comment:3 Changed 7 years ago by tscrim

  • Dependencies changed from #14772 to #14772 #13589

There is a minor dependency in integer_vector_weighted.py from #13589.

comment:4 Changed 7 years ago by tscrim

  • Dependencies changed from #14772 #13589 to #14772 #13589 #14907

Removed fuzz caused by #14907 (I think).

comment:5 Changed 7 years ago by tscrim

I also fixed the error with SkewTableau.to_chain().

comment:6 Changed 7 years ago by zabrocki

  • Cc zabrocki added

comment:7 Changed 7 years ago by tscrim

Fixed corner case of to_chain(). Thanks for catching that Mike.

Changed 7 years ago by darij

still unfinished review patch

comment:8 Changed 7 years ago by darij

OOPS wrong thread, sorry!

comment:9 Changed 7 years ago by tscrim

For patchbot:

Apply: trac_14101-remove_cc_skew-ts.patch

comment:10 Changed 7 years ago by alubovsky

  • Status changed from needs_review to positive_review

comment:11 Changed 7 years ago by darij

1) Why does this depend on #14907?

2) 

sage -t devel/sage-main/sage/combinat/ribbon_tableau.py
**********************************************************************
File "devel/sage-main/sage/combinat/ribbon_tableau.py", line 201, in sage.combinat.ribbon_tableau.RibbonTableaux
Failed example:
    for i in R: i.pp(); print
Expected:
      .  .  0  0  0
      .  0  0  2
      1  0  1
Got:
      .  .  0  0  0
      .  0  0  2
      1  0  1
    <BLANKLINE>
      .  .  1  0  0
      .  0  0  0
      1  0  2
    <BLANKLINE>
      .  .  0  0  0
      .  1  0  1
      2  0  0
    <BLANKLINE>
**********************************************************************

This seems to be a bug with the doctesting framework, which thinks of empty lines as EOM. Just replacing "print" by "print 'some_nonempty_string'" does the trick.

Last edited 7 years ago by darij (previous) (diff)

comment:12 Changed 7 years ago by darij

  • Status changed from positive_review to needs_work

comment:13 Changed 7 years ago by tscrim

  • Description modified (diff)
  • Status changed from needs_work to needs_review

Fixed the above (all one has to do was add <BLANKLINE>) and added the comment that Arthur and I talked about via email.

For patchbot:

Apply: trac_14101-remove_cc_skew-ts.patch

comment:14 Changed 7 years ago by darij

The doctest works now; positive review then?

comment:15 Changed 7 years ago by tscrim

Arthur had wanted me to change some documentation, so he'll set the positive review once he's looked that over.

comment:16 Changed 7 years ago by alubovsky

  • Status changed from needs_review to positive_review

comment:17 Changed 7 years ago by tscrim

Thanks for doing the final review Arthur.

comment:18 Changed 7 years ago by alubovsky

Travis,

No problem. Thanks for your help.

comment:19 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:20 Changed 7 years ago by jferreira

  • Status changed from positive_review to needs_work

There is fuzz applying to 5.11.beta3 

applying trac_14101-remove_cc_skew-ts.patch
patching file sage/combinat/tableau.py
Hunk #10 succeeded at 1486 with fuzz 2 (offset -52 lines).
now at: trac_14101-remove_cc_skew-ts.patch

None of the dependencies touch tableau.py. Can you please rebase to see if that will fix the fuzz?

Thanks, Jeff

comment:21 Changed 7 years ago by tscrim

  • Dependencies changed from #14772 #13589 #14907 to #7983 #14772 #13589 #14907
  • Status changed from needs_work to positive_review

Try it with #7983. I'm not getting any fuzz (in the combinat queue).

Last edited 7 years ago by tscrim (previous) (diff)

comment:22 Changed 7 years ago by jferreira

Travis,

Yes that worked. It now applies without any fuzz. Thanks.

Jeff

comment:23 Changed 7 years ago by tscrim

Thanks for catching that Jeff.

comment:24 Changed 7 years ago by jferreira

  • Status changed from positive_review to needs_work

I get a warning when building the doc: 

[combinat ] /Applications/sage/local/lib/python2.7/site-packages/sage/combinat/ribbon_tableau.py:docstring of sage.combinat.ribbon_tableau.RibbonTableau:23: WARNING: Literal block expected; none found.

comment:25 Changed 7 years ago by tscrim

  • Status changed from needs_work to positive_review

I've fixed the offending docstring.

For patchbot:

Apply: trac_14101-remove_cc_skew-ts.patch

comment:26 Changed 7 years ago by jdemeyer

  • Dependencies changed from #7983 #14772 #13589 #14907 to #7983, #14772, #13589, #14907, #10630
  • Milestone changed from sage-5.12 to sage-pending

comment:27 Changed 7 years ago by tscrim

  • Milestone changed from sage-pending to sage-5.12

Changed 7 years ago by tscrim

comment:28 Changed 7 years ago by tscrim

Trivial rebase over changes in sources.py in #14772.

comment:29 Changed 7 years ago by jdemeyer

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