Opened 16 months ago

Closed 15 months ago

Last modified 15 months ago

#26391 closed enhancement (fixed)

py3: partial cure for multiset_partition_into_sets_ordered

Reported by: chapoton Owned by:
Priority: major Milestone: sage-8.5
Component: python3 Keywords:
Cc: Merged in:
Authors: Frédéric Chapoton Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: a4921a4 (Commits) Commit: a4921a4421a92fa9d7785f602d6cf4e163c5159a
Dependencies: Stopgaps:

Description

which behaves very badly with python3... :(

Change History (10)

comment:1 Changed 16 months ago by chapoton

  • Branch set to u/chapoton/26391
  • Commit set to d13b97b053d69f7d01be63e5f1153af980c498af
  • Status changed from new to needs_review

New commits:

​d13b97bpy3: partial fix for multiset partitions

comment:2 Changed 16 months ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

Sorry about having to make you fix those things by not catching them on review.

comment:3 Changed 16 months ago by vbraun

  • Status changed from positive_review to needs_work

On OSX: 

**********************************************************************
File "src/sage/combinat/multiset_partition_into_sets_ordered.py", line 2003, in sage.combinat.multiset_partition_into_sets_ordered.OrderedMultisetPartitionsIntoSets_all_constraints._repr_
Failed example:
    OrderedMultisetPartitionsIntoSets(min_length=3, max_order=5, alphabet=[1,'a'])
Expected:
    Ordered Multiset Partitions into Sets with constraints:
     alphabet={1, 'a'}, max_order=5, min_length=3
Got:
    Ordered Multiset Partitions into Sets with constraints: alphabet={'a', 1}, max_order=5, min_length=3
**********************************************************************
1 item had failures:
   1 of   3 in sage.combinat.multiset_partition_into_sets_ordered.OrderedMultisetPartitionsIntoSets_all_constraints._repr_
    [570 tests, 1 failure, 32.46 s]
----------------------------------------------------------------------
sage -t --long src/sage/combinat/multiset_partition_into_sets_ordered.py  # 1 doctest failed
----------------------------------------------------------------------

comment:4 Changed 16 months ago by tscrim

I missed this before, but this A = sorted(cdict["alphabet"]) will break on Python3 in the test above.

comment:5 Changed 16 months ago by tscrim

Probably what needs to be done is 

if not all(l in ZZ for l in cdict["alphabet"]):
    A = sorted(cdict["alphabet"], key=str)
else:
    A = sorted(cdict["alphabet"]

comment:6 Changed 16 months ago by git

  • Commit changed from d13b97b053d69f7d01be63e5f1153af980c498af to a4921a4421a92fa9d7785f602d6cf4e163c5159a

Branch pushed to git repo; I updated commit sha1. New commits:

​ad09b20Merge branch 'u/chapoton/26391' in 8.4.rc0
​a4921a4trac 26391 one fix

comment:7 Changed 16 months ago by chapoton

  • Status changed from needs_work to needs_review

comment:8 Changed 16 months ago by tscrim

  • Status changed from needs_review to positive_review

Thanks. LGTM.

comment:9 Changed 15 months ago by vbraun

  • Branch changed from u/chapoton/26391 to a4921a4421a92fa9d7785f602d6cf4e163c5159a
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:10 Changed 15 months ago by embray

  • Milestone changed from sage-8.4 to sage-8.5

This should be re-targeted for 8.5.

Note: See TracTickets for help on using tickets.