Opened 6 years ago

Closed 6 years ago

#14081 closed defect (fixed)

Solve bug in BraidGroup

Reported by: mmarco Owned by: joyner
Priority: major Milestone: sage-5.9
Component: group theory Keywords:
Cc: jdemeyer, vbraun Merged in: sage-5.9.beta4
Authors: Miguel Marco Reviewers: Travis Scrimshaw, Volker Braun
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

BraidGroup? checks that the number of generators is bigger or equal than 2. It should be 1 (two strands, one generator). So, now the braid group on two strands can't be created.

This patch solves it. (It is just a one line change):

Apply: 14081_one_generator.patch​ and 14081_one_generator2.patch​

Attachments (2)

14081_one_generator.patch​ (977 bytes) - added by mmarco 6 years ago.
14081_one_generator2.patch​ (905 bytes) - added by mmarco 6 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 6 years ago by tscrim

  • Authors set to Miguel Marco
  • Reviewers set to Travis Scrimshaw

Could you add a doctest showing this is fixed and a comment on that about about why it should be n < 1 instead of n < 2 as the error message (correctly) indicates (such as # n now is the number of generators)? Once that is done, set this to needs_review.

Thanks,
Travis

Changed 6 years ago by mmarco

comment:2 Changed 6 years ago by mmarco

  • Status changed from new to needs_review

Apply 14981_one_generator.patch

comment:3 Changed 6 years ago by vbraun

  • Description modified (diff)

comment:4 Changed 6 years ago by vbraun

For the patchbot:

Apply 14081_one_generator.patch

comment:5 Changed 6 years ago by tscrim

  • Description modified (diff)

There's also a doctest failure (due to capitalization). Also, for the doctest, could you make it some form of: 

Check that :trac:`14081` is fixed::

    sage: BraidGroup(2)
    Braid group on 2 strands
    sage: BraidGroup(('a',))
    Braid group on 2 strands

Thanks.

Changed 6 years ago by mmarco

comment:6 Changed 6 years ago by mmarco

  • Description modified (diff)

comment:7 Changed 6 years ago by vbraun

  • Reviewers changed from Travis Scrimshaw to Travis Scrimshaw, Volker Braun
  • Status changed from needs_review to positive_review

Looks good to me

comment:8 Changed 6 years ago by tscrim

Same. Thank you for your work Miguel and the final review Volker.

comment:9 Changed 6 years ago by jdemeyer

  • Description modified (diff)

comment:10 Changed 6 years ago by jdemeyer

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