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

py3: partial cure for multiset_partition_into_sets_ordered #26391

Closed
fchapoton opened this issue Oct 3, 2018 · 13 comments
Closed

py3: partial cure for multiset_partition_into_sets_ordered #26391

fchapoton opened this issue Oct 3, 2018 · 13 comments

Comments

@fchapoton
Copy link
Contributor

which behaves very badly with python3... :(

Component: python3

Author: Frédéric Chapoton

Branch/Commit: a4921a4

Reviewer: Travis Scrimshaw

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

@fchapoton fchapoton added this to the sage-8.4 milestone Oct 3, 2018
@fchapoton
Copy link
Contributor Author

Branch: u/chapoton/26391

@fchapoton
Copy link
Contributor Author

Commit: d13b97b

@fchapoton
Copy link
Contributor Author

New commits:

d13b97bpy3: partial fix for multiset partitions

@tscrim
Copy link
Collaborator

tscrim commented Oct 3, 2018

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Oct 3, 2018

comment:2

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

@vbraun
Copy link
Member

vbraun commented Oct 5, 2018

comment:3

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
----------------------------------------------------------------------

@tscrim
Copy link
Collaborator

tscrim commented Oct 5, 2018

comment:4

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

@tscrim
Copy link
Collaborator

tscrim commented Oct 5, 2018

comment:5

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"]

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 8, 2018

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

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 8, 2018

Changed commit from d13b97b to a4921a4

@tscrim
Copy link
Collaborator

tscrim commented Oct 10, 2018

comment:8

Thanks. LGTM.

@vbraun
Copy link
Member

vbraun commented Oct 20, 2018

Changed branch from u/chapoton/26391 to a4921a4

@embray
Copy link
Contributor

embray commented Oct 28, 2018

comment:10

This should be re-targeted for 8.5.

@embray embray modified the milestones: sage-8.4, sage-8.5 Oct 28, 2018
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