Opened 6 years ago

Closed 6 years ago

#14583 closed defect (fixed)

Permutation('()') and Permutation(''), each in its own way, fail to return the identity in S_0

Reported by: darij Owned by: sage-combinat
Priority: major Milestone: sage-5.10
Component: combinatorics Keywords: combinat
Cc: sage-combinat, tscrim, nthiery Merged in: sage-5.10.beta4
Authors: Darij Grinberg Reviewers: Nathann Cohen
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by darij)

I brought this up in #8392 but was told to post it as a new ticket.

sage: Permutation([])  # This is be the identity permutation in S_0.
[]
sage: Permutation([]).cycle_string()    # OK.
'()'
sage: Permutation('()')    # This should give the S_0 identity back -- but it doesn't.
[1]
sage: Permutation('')     # Does this maybe? No.
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-29-3df27d9d4d7a> in <module>()
----> 1 Permutation('')

/home/darij/sage-5.10.beta2/local/lib/python2.7/site-packages/sage/combinat/permutation.pyc in Permutation(l, check_input)
    430         cycle_list = []
    431         for c in cycles:
--> 432             cycle_list.append(map(int, c.split(",")))
    433 
    434         return from_cycles(max([max(c) for c in cycle_list]), cycle_list)

ValueError: invalid literal for int() with base 10: ''
sage: Permutation(())       # What about this?
[1]

A corner case it is, but I think it should be done right...

This is now fixed in the attached patch.

Attachments (2)

trac_14583-s0-v1.patch (2.1 KB) - added by darij 6 years ago.
fixes for a couple of these bugs (not sure there aren't any others)
trac_14583-s0-v2.patch (6.8 KB) - added by darij 6 years ago.
new version

Download all attachments as: .zip

Change History (10)

comment:1 Changed 6 years ago by darij

  • Description modified (diff)

comment:2 Changed 6 years ago by darij

  • Description modified (diff)

Changed 6 years ago by darij

fixes for a couple of these bugs (not sure there aren't any others)

comment:3 Changed 6 years ago by darij

  • Authors set to Darij Grinberg
  • Cc tscrim nthiery added
  • Description modified (diff)
  • Status changed from new to needs_review

comment:4 Changed 6 years ago by ncohen

  • Status changed from needs_review to needs_info

Helloooooooooooooooooooo !!

Make sense ! Though shouldn't if len(flattened_and_sorted) == 0: in from_cycles return the identity on n elements instead ? If the user asked for n elements, after all...

Nathann

Changed 6 years ago by darij

new version

comment:5 Changed 6 years ago by darij

Of course! Thanks for catching this one.

I fixed this and a few more trifles in the present version.

comment:6 Changed 6 years ago by darij

  • Description modified (diff)
  • Reviewers set to ncohen
  • Status changed from needs_info to needs_review

comment:7 Changed 6 years ago by ncohen

  • Reviewers changed from ncohen to Nathann Cohen
  • Status changed from needs_review to positive_review

The patch makes sense, and passes all tests !

Nathann

comment:8 Changed 6 years ago by jdemeyer

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